feat: solana balances #19
Conversation
There was a problem hiding this comment.
Looks good in general, but I think having 1 hour of discovering new ALEPH holders will be too frustrating to new users wanting to stake tokens.
Some alternatives I could see, prerrably the first ones:
- Find out when people create an $ALEPH ATA instantly and create a latest snapshot from it.
- Force refresh all $ALEPH holders when calling the
balancesendpoint (or add a parameter to request the latest balances)
| export const ERC20Balance = new GraphQLObjectType({ | ||
| name: 'ERC20Balance', | ||
| export const Balance = new GraphQLObjectType({ | ||
| name: 'Balance', |
There was a problem hiding this comment.
This should be still a ERC20Balance, as we have a Solana specific implementation down below
There was a problem hiding this comment.
balance query also can return solana balances from snapshot db
There was a problem hiding this comment.
I see now where the problem in this discussion lies.
What you can do is make Balance a union of two types, ERC20Balance and SolanaBalance:
const erc20BalanceFields = {
account: { type: new GraphQLNonNull(GraphQLString) },
balance: { type: new GraphQLNonNull(GraphQLBigNumber) },
balanceNum: { type: GraphQLFloat },
}
const solanaBalanceFields = {
...erc20BalanceFields,
tokenAccount: { type: new GraphQLNonNull(GraphQLString) },
}
export const ERC20Balance = new GraphQLObjectType({
name: 'ERC20Balance',
fields: erc20BalanceFields,
})
export const SolanaBalance = new GraphQLObjectType({
name: 'SolanaBalance',
fields: solanaBalanceFields,
})
export const Balance = new GraphQLUnionType({
name: 'Balance',
types: [ ERC20Balance, SolanaBalance],
});This will allow you to keep all the relevant info for Solana-related balances, while not having to deal with undefined values for ERC20Balances. This style also comes in handy later, when you might want to filter balances based on which type they are.
| account: { type: new GraphQLNonNull(GraphQLString) }, | ||
| owner: { type: new GraphQLNonNull(GraphQLString) }, |
There was a problem hiding this comment.
Comparing this to the previous ERC20Balance, the account fields mean two different things:
- For ERC20 it is the owner account
- For Solana is is the ATA
For clarification I'd prefer if the owner account stays the account and instead we call the ATA the tokenAccount
There was a problem hiding this comment.
okay i can change that, then balances query should return tokenAccount or owner? it seems more straightforward to return the tokenAccount as this is where the tokens are stored. I can implement a query to return token account data also.
There was a problem hiding this comment.
tokenAccount is an implementation detail that is not relevant to (most) users. Provide the information, but don't make it the primary identifier. The owner address is much more relevant to most usecases.
| }, | ||
| }) | ||
|
|
||
| export const types = [TransferEvent, Balance, Snapshot] |
There was a problem hiding this comment.
Solana types are not being exported in this list, is this intended?
There was a problem hiding this comment.
Snapshot is the 'solana type', balances query also returns solana balances data
There was a problem hiding this comment.
Feels a bit off to me... Are ERC20 balances not capable of snapshots? Why even collect snapshots then, if we could just try to index the current balance at any time?
| @@ -49,7 +49,7 @@ const mapValueFn = async (value: any) => { | |||
|
|
|||
| export function createBalanceDAL(path: string): BalanceStorage { | |||
| return new EntityStorage<Balance>({ | |||
| name: 'erc20_balance', | |||
| name: 'balance', | |||
There was a problem hiding this comment.
So we are storing Balance (previously ERC20Balance) in this database. What about Solana balances and their additional field then? Are they not stored?
There was a problem hiding this comment.
The snapshot taken is stored on the snapshot dal
| if (!accountBalance) { | ||
| const balance = blockchainTotalSupply[blockchain].toString('hex') | ||
| await this.balanceDAL.save({ blockchain, account: deployer, balance }) | ||
| if (!meta) { |
There was a problem hiding this comment.
Token contracts are indexed without any meta, inside the condition is executed code related to token contracts
There was a problem hiding this comment.
Would be good to comment on that. Had no idea.
packages/aleph-holders/src/types.ts
Outdated
| account: string | ||
| owner: string |
There was a problem hiding this comment.
Same here, the owner is more similar to the account field of the ERC20Balance
| // note: Snapshot interval is set to 1 hour | ||
| protected discoveryInterval = 1000 * 60 * 60, |
There was a problem hiding this comment.
This feels way too long in my opinion. Is there no way to instantly recognize newly created ATAs? When users add funds to their wallet and want to stake, they expect our indexers to instantly recognize the added tokens.
I have used the discoverAccounts implementation, which is prepared to set an interval to run a job, the holders snapshot is taken every hour.
The balance query, if Solana is requested, returns the last snapshot, but it is also possible to query other snapshots taken giving a timestamp, will return last snapshot of that timestamp.