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.76k stars 1.55k forks source link

"Phantom" multisig approvals appearing on Accounts page #9103

Open laboon opened 1 year ago

laboon commented 1 year ago

I am seeing "phantom" multisig approvals that don't exist on accounts, and in fact are on some accounts that are not multisigs. When you click on them, a rendering error is displayed.

Uncaught error. Something went wrong with the query and rendering of this component. Please supply all the details below when logging an issue, it may help in tracing the cause.

Cannot read properties of undefined (reading 'map')

TypeError: Cannot read properties of undefined (reading 'map')
at https://polkadot.js.org/apps/page.380411d44a77a4fd.js:339:15657
at rs (https://polkadot.js.org/apps/modu.d8a2c6c99e89d217.js:2:98954)
at Cu (https://polkadot.js.org/apps/modu.d8a2c6c99e89d217.js:2:119076)
at https://polkadot.js.org/apps/modu.d8a2c6c99e89d217.js:2:117818
at wu (https://polkadot.js.org/apps/modu.d8a2c6c99e89d217.js:2:117883)
at su (https://polkadot.js.org/apps/modu.d8a2c6c99e89d217.js:2:111639)
at Ho (https://polkadot.js.org/apps/modu.d8a2c6c99e89d217.js:2:52410)
at https://polkadot.js.org/apps/modu.d8a2c6c99e89d217.js:2:109044

Refreshing the screen does not make them go away, but changing endpoints seems to do so.

At least one other person has had this happen to them. I am happy to provide specific addresses privately, if it will help debug the issue.

Screenshot (with addresses obscured):

Screenshot 2023-03-03 at 18 09 29
michalisFr commented 1 year ago

We're seeing something similar with the anti-scam bounty multisig. We see a pending call that is for a different multisig.

Screenshot 2023-03-06 at 3 38 34 PM

The multisig call we see is this one: https://polkadot.subscan.io/extrinsic/14531018-2

This behaviour happens when connected to the following nodes:

kianenigma commented 1 year ago
Screenshot 2023-03-08 at 18 15 33

Also seeing this anomaly in Rococo, the key1 of the double map is not matching the query, which is wrong right of the bat.

Reported in https://substrate.stackexchange.com/questions/7505/multisig-storage-showing-calls-from-another-one.

MarkEmberRyan commented 1 year ago

We are also seeing this anomoly in the Events Bounty multisig.

For us, it appears to be related to whether or not there is a valid pending hash in the queue - when we submit a valid multisig call, the phantom hash disappears. When the valid pending call is signed and cleared, the phantom hash returns. Always the same hash, always the same depositor address submitting the phantom call. The activity of the phantom depositor is not reflected on Subscan.

As others stated above, the presence of this anomaly also appears related to which RPC is selected.

We'd be happy to provide the address of the phantom depositor as well if this would be helpful.

jacogr commented 1 year ago

If this is (still) an issue I am sadly missing vital info to take a look. (I am not seeing this on any of my daily use multisigs)

If it happens all the time -

laboon commented 1 year ago

It is unfortunately still an issue. We are also seeing the same call hashes on the same accounts across different users.

I can email you the accounts I am seeing this on, along with chain and endpoint (and call hashes), unless you prefer a different way for me to privately share information (e.g. Matrix). Just let me know.

jacogr commented 1 year ago

The call hash doesn't make a difference here - the UI doesn't "inject details", it only queries the entries of the multisig.multisigs map and displays that info as-is. (It also doesn't re-filter the retrieved details by key, i.e. it would expect to receive as queried)

If nobody volunteers an actual multisig address (the more, the merrier - alongside chain/endpoint), will most probably look you up on Matrix.

Tbaut commented 1 year ago

I posted 2 multisig addresses with this issue in the stack exchange link above, that's on Rococo: https://polkadot.js.org/apps/?rpc=wss%3A%2F%2Frococo-rpc.polkadot.io#/chainstate

This is definitely coming from the chain. @kianenigma had a quick look at the pallet but I don't think anything has changed recently.

jacogr commented 1 year ago

Yes, seems to be coming from the RPC node on the above 2 -

image

Also tested an old version of the JS API via cli, same results -

import { ApiPromise, WsProvider } from '@polkadot/api';

async function main () {
    const api = await ApiPromise.create({
        provider: new WsProvider('wss://rococo-rpc.polkadot.io')
    });
    const entries = await api.query.multisig.multisigs.entries('5FRz8z6U87LBXLUuBRRy4JtVAh2jhkDSbR5ntCGM2Ds631dJ');

    console.log(entries.map(([k]) => k.toHuman()));
}

main().catch(console.error);
jacogr commented 1 year ago

Also checked the .keys methods, same result, i.e. wrong entries returned.

So that basically means that state_getKeysPaged returns wrong entries, so need to start digging through Substrate PRs, e.g. this is a a recent 2-week-old & 3-week-old ones - https://github.com/paritytech/substrate/pull/13445 & https://github.com/paritytech/substrate/pull/13284 (deals with storage iteration)

It won't be in the pallet itself and that is the crux of the issue, it is RPC related - the UI can plaster over the "multisig issue" by filtering on keys (it never should need to, returned should be for what is queried), but if storage iteration is not working, it means it has wide effects which we just have not seen, but is there.

So on Polkadot -

it possibly helps dissecting.

TL:DR Based on queries via ancient API versions, can only point to some recent RPC-related changes.

bkchr commented 1 year ago

Okay, maybe we are iterating too far or including at start something that shouldn't be included.

Cc @koute

koute commented 1 year ago

I'll check this.

koute commented 1 year ago

This was indeed a bug in Substrate; to anyone interested here's a PR with a fix: https://github.com/paritytech/substrate/pull/13630