Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fetch balances from Oasis gRPC instead of Web3 RPC #1384

Merged
merged 4 commits into from
Apr 15, 2024
Merged

Conversation

lukaw3d
Copy link
Member

@lukaw3d lukaw3d commented Apr 15, 2024

Fixes #1367

Differences in behaviour:

  • Fetches all balances, not just the native one
  • Fetches balances for contracts
  • Fetches balances for all accounts, not just EVM ones
  • Starts the request in parallel with Nexus API call

Related to #1073 #1338 #1374 #1318 (comment)

Copy link

github-actions bot commented Apr 15, 2024

Deployed to Cloudflare Pages

Latest commit: 595cecac1139a08547f03ba85bd8c2ef4e16b911
Status:✅ Deploy successful!
Preview URL: https://8d8ae2aa.oasis-explorer.pages.dev

src/app/utils/getRPCAccountBalances.ts Show resolved Hide resolved
src/app/utils/getRPCAccountBalances.ts Show resolved Hide resolved
return {
balance: oasis.quantity.toBigInt(amount).toString(),
token_symbol: oasis.misc.toStringUTF8(denomination) || getTokensForScope(scope)[0].ticker,
token_decimals: paraTimesConfig[scope.layer].decimals, // TODO: differentiate by token
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That doesn't look right, if this is querying ERC-20 tokens as well, each of the ERC-20 token, can have custom decimals specified. So defaulting to paratime decimals here, is wrong.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's only querying e.g. TEST and EUROe; evm_balances are a different field in Nexus API.
But we don't differentiate decimals between TEST and EUROe - related: #1265 (comment)

Copy link
Collaborator

@lubej lubej Apr 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I verified that both Ocean and EUROe use 18 decimals places, so coincidentally this works as expected.

Looking at the documentation at https://docs.pontus-x.eu/docs/Development/quick_start#setup-metamask the native token has been renamed to GX @csillag are we aware of this?

Scratch the above, was looking at the wrong documentation. The right documentation is linked here: https://docs.pontus-x.eu/docs/ParaTime/quick_start#setup-metamask

Comment on lines +41 to +46
.map(token => {
return {
...token,
balance: fromBaseUnits(token.balance, token.token_decimals),
}
})
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is iterating twice over an array, for no good reason. This should happen in the previous map.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer this, and it mirrors

.map(token => {
return {
...token,
balance: fromBaseUnits(token.balance, token.token_decimals),
}
}),
.

(I also think https://github.com/oasisprotocol/explorer-frontend/blob/26079807ff9a4e91cc42d106d75a0c824533caba/src/oasis-nexus/api.ts#L709-L752 should be split into multiple iterations over an array)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it is not a large array, but for me it makes no sense, to iterate over the same array multiple times.

This fixes wrong balance on non-EVM addresses like fee accumulator. It also gets
balances of all tokens (e.g. TEST and EUROe on Pontus-X). And it can start the
request before Nexus responds with address_eth.
@lukaw3d lukaw3d merged commit 165caa2 into master Apr 15, 2024
8 checks passed
@lukaw3d lukaw3d deleted the lw/rpc-refactor branch April 15, 2024 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Gas fee paying address on Sapphire testnet showing wrong amount
3 participants