polkadot-js / apps

Basic Polkadot/Substrate UI for interacting with a Polkadot and Substrate node. This is the main user-facing application, allowing access to all features available on Substrate chains.
https://dotapps.io
Apache License 2.0
1.74k stars 1.53k forks source link

Displaying account balances on a custom chain #4078

Closed tomusdrw closed 3 years ago

tomusdrw commented 3 years ago

I presume account balance on the balances tab is coming from frame_system storage.

For some cases, when the chain supports multiple tokens there are two instances of balances pallet and neither should be favoured, like here: https://gitlab.com/interlay/btc-parachain/-/blob/dev/parachain/runtime/src/lib.rs#L267

Is there a way to configure the UI to display balances from different AccountStorage?

To test, you can connect to: wss://alpha.polkabtc.io/api/parachain

and use these custom types
{
 "H256Le": "H256",
    "DOT": "u128",
    "PolkaBTC": "Balance",
    "BTCBalance": "u128",
    "StatusCode": {
      "_enum": ["Running", "Error", "Shutdown"]
    },
    "Address": "AccountId",
    "LookupSource": "AccountId",
    "Status": "StatusCode",
    "ErrorCode": {
      "_enum": [
        "None",
        "NoDataBTCRelay",
        "InvalidBTCRelay",
        "OracleOffline",
        "Liquidation"
      ]
    },
    "VaultStatus": {
      "_enum": ["Active", "Liquidated", "CommittedTheft"]
    },
    "RawBlockHeader": {
      "0": "[u8; 80]"
    },
    "RichBlockHeader": {
      "block_hash": "H256Le",
      "block_header": "BlockHeader",
      "block_height": "u32",
      "chain_ref": "u32"
    },
    "BlockHeader": {
      "merkle_root": "H256Le",
      "target": "U256",
      "timestamp": "u32",
      "version": "i32",
      "hash_prev_block": "H256Le",
      "nonce": "u32"
    },
    "BlockChain": {
      "chain_id": "u32",
      "start_height": "u32",
      "max_height": "u32",
      "no_data": "BTreeSet",
      "invalid": "BTreeSet"
    },
    "Wallet": {
      "addresses": "BTreeSet",
      "address": "H160"
    },
    "Vault": {
      "id": "AccountId",
      "to_be_issued_tokens": "PolkaBTC",
      "issued_tokens": "PolkaBTC",
      "to_be_redeemed_tokens": "PolkaBTC",
      "wallet": "Wallet",
      "banned_until": "Option",
      "status": "VaultStatus"
    },
    "ActiveStakedRelayer": {
      "stake": "DOT"
    },
    "StatusUpdate": {
      "new_status_code": "StatusCode",
      "old_status_code": "StatusCode",
      "add_error": "Option",
      "remove_error": "Option",
      "start": "BlockNumber",
      "end": "BlockNumber",
      "proposal_status": "ProposalStatus",
      "btc_block_hash": "Option",
      "proposer": "AccountId",
      "deposit": "DOT",
      "tally": "Tally",
      "message": "Text"
    },
    "Tally": {
      "aye": "BTreeSet",
      "nay": "BTreeSet"
    },
    "ProposalStatus": {
      "_enum": ["Pending", "Accepted", "Rejected", "Expired"]
    },
    "InactiveStakedRelayer": {
      "stake": "DOT",
      "status": "StakedRelayerStatus"
    },
    "StakedRelayerStatus": {
      "_enum": ["Unknown", "Idle", "Bonding(BlockNumber)"]
    },
    "IssueRequest": {
      "vault": "AccountId",
      "opentime": "BlockNumber",
      "griefing_collateral": "DOT",
      "amount": "PolkaBTC",
      "requester": "AccountId",
      "btc_address": "H160",
      "completed": "bool"
    },
    "RedeemRequest": {
      "vault": "AccountId",
      "opentime": "BlockNumber",
      "amount_polka_btc": "PolkaBTC",
      "amount_btc": "PolkaBTC",
      "amount_dot": "DOT",
      "premium_dot": "DOT",
      "redeemer": "AccountId",
      "btc_address": "H160"
    },
    "ReplaceRequest": {
      "old_vault": "AccountId",
      "open_time": "BlockNumber",
      "amount": "PolkaBTC",
      "griefing_collateral": "DOT",
      "new_vault": "Option",
      "collateral": "DOT",
      "accept_time": "Option",
      "btc_address": "H160"
    },

    "AccountIdJsonRpcResponse": {
      "account_id": "String"
    },
    "RegisterStakedRelayerJsonRpcRequest": {
      "stake": "u128"
    },
    "SuggestStatusUpdateJsonRpcRequest": {
      "deposit": "u128",
      "status_code": "StatusCode",
      "add_error": "Option",
      "remove_error": "Option",
      "block_hash": "Option",
      "message": "String"
    },
    "VoteOnStatusUpdateJsonRpcRequest": {
      "status_update_id": "U256",
      "approve": "bool"
    },
    "ReplaceRequestJsonRpcRequest": {
      "amount": "u128",
      "griefing_collateral": "u128"
    },
    "RegisterVaultJsonRpcRequest": {
      "collateral": "u128",
      "btc_address": "H160"
    },
    "ChangeCollateralJsonRpcRequest": {
      "amount": "u128"
    },
    "UpdateBtcAddressJsonRpcRequest": {
      "address": "H160"
    },
    "WithdrawReplaceJsonRpcRequest": {
      "replace_id": "H256"
    },
    "BalanceWrapper": {
      "amount": "String"
    },
    "BtcTxFeesPerByte": {
      "fast": "u32",
      "half": "u32",
      "hour": "u32"
    }
}
jacogr commented 3 years ago

No, it is not supported, there is a fallback strategy that gets applied and a single storage item is used - https://github.com/polkadot-js/api/blob/master/packages/api-derive/src/balances/account.ts#L89-L93

For instance on Kusama/Polkadot, the balances.account also actually does exist, it just returns the default (i.e. all 0) value. So query preference, as linked above, is system.account (if it exists), then balances.account (if it exists and the prior doesn't) then balances.<allOldStuff>

daniel-savu commented 3 years ago

We've recently encountered this problem again, where the Polkadot explorer fails to display PolkaBTC account balances (https://polkadot.js.org/apps/#/accounts).

Tomasz helped point out that there is a type mismatch that is likely the source of the problem.

Substrate:

type AccountStore = frame_system::Module<Runtime>;

BTC-Parachain:

type AccountStore = StorageMapShim<
        pallet_balances::Account<Runtime, pallet_balances::Instance1>,
        frame_system::CallOnCreatedAccount<Runtime>,
        frame_system::CallKillAccount<Runtime>,
        AccountId,
        pallet_balances::AccountData<Balance>,
    >;

@jacogr What do you suggest we could do?

jacogr commented 3 years ago

The Apps UI is not configurable atm for this, i.e. it will only look at the locations above.

There is nothing that stands in the way for adding this as a config (it does have some gremlins since it dives down into the api-derives, e.g. https://github.com/polkadot-js/api/blob/master/packages/api-derive/src/balances/account.ts is where the balances are actually retrieved).

Additionally, there are a number of chains with 2 types of balances now, so that also needs to tie in and you really don't want to display "the one".

The long and the short of it - It is not something that was designed up-front, but at the same time is nowhere close to "impossible". With config-params-to-the-API it can be a possibility. Happy for PRs in this direction, as mentioned the issue actually lies in the derive layer which is meant to "get balances from anywhere, no matter how configured" - which it only does reliably for "standard" Substrate setups atm.

PS: It affects a lot more that balances actually, balances just being more prominent, for instance you can have multiple councils, treasuries, etc.

jacogr commented 3 years ago

Do you have any account with balances on PolkaBTC I could take a look at. (Always difficult when you don't have anything). Preferably something with polkaBtc and dot or either-or.

Will take a look and the impact of changes - if simple can squeeze it in somewhere, alternatively can at least type something as to what it will/can take.

daniel-savu commented 3 years ago

The Alice account (5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY) has both DOT and PolkaBTC. Thanks!

jacogr commented 3 years ago

Ok, after #4477 the dot balances should show. I would suggest you update the interlay types to contain the following instead of the apps UI manually hacking it in. See the addition at https://github.com/polkadot-js/apps/pull/4477/files#diff-17ed0ea2987b708fce0919274597c03092e6b197397d7516888bc64ee270dcb0R12 which translates to the following in the published types -

"instances": {
  "balances": ["dot", "polkaBtc"]
},
"types": { ... }

That allows the API to know where the balances lives so it can actually query all of them. The BTC balances display will come, it is actually returned already by the API, but the UI ignores that part for now. (It is still unknown to it and needs some TC to figure out how/where to properly display multiples)

Additionally with polkadot-js/api#3093 it will allow you to actually specify the token decimals and well as token for both in the chainspec, currently these are not defined at all by interlay, i.e. the system.properties RPC returns nothing, hence the "Unit" naming. (currently it only allows a single, the above PR allows support for multiples in terms of symbols and decimals). There is still quite a bit of TLC though to get that formatting right, the first one will be easy the second balance a bit trickier.

daniel-savu commented 3 years ago

Awesome news! I'll update the types the include the instances and remove the hardcoded version from apps on Monday in a PR. I'm not very involved in parachain development but I'll try to update system.properties soon too

jacogr commented 3 years ago

Yes, both should be fine Monday(API releases go each Monday so it will hit mainstream then - apps UI is always bleeding-edge with betas)

jacogr commented 3 years ago

For system.properties, it is defined in the chainspec, see for instance Kusma, https://raw.githubusercontent.com/paritytech/polkadot/master/node/service/res/kusama.json - which is -

"properties": {
    "ss58Format": 2,
    "tokenDecimals": 12,
    "tokenSymbol": "KSM"
  },

So the support the API adds is for the following form -

"properties": {
    "ss58Format": 42,
    "tokenDecimals": [10, 8],
    "tokenSymbol": ["DOT", "BTC"]
  },

(For Rococo it is probably [12, 8] & ["ROC", "BTC"] - or your own token name, whatever you believe is correct for your case, it really is display-only.

jacogr commented 3 years ago

Closing. There are a couple of issues logged (API doesn't respect the settings in e.g. state display, transfers needs to be available) that makes it more usable, but as a first round the display is in place.

jacogr commented 3 years ago

@savudani8 Like it -

image

I'll get to the transfer issue soon, just have not had a gap.

daniel-savu commented 3 years ago

Thank you! 😊

polkadot-js-bot commented 3 years ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue if you think you have a related problem or query.