LightClient storage iterator returns duplicates

[ggwpez]

I am using the light client feature to try and iterate over all members of the Fellowship.
Code looks like this:

let key = collectives::storage().fellowship_collective().members_iter();
let mut query = parachain_api.storage().at_latest().await.unwrap().iter(key).await?;

while let Some(Ok((id, fellow))) = query.next().await {
    // ..
}

But somehow it returns duplicate values. The full code is on this branch: master...oty-light-client-iter-duplicate
Note that it does work when using an RPC node.

[niklasad1]

Thanks for the issue, we'll have a look at it soon.

@lexnv I guess you may have some hunch about this one :)

[lexnv]

I was able to reproduce this locally. My first hunch is that smoldot is providing us duplicate answers.

The duplication is checked by account ID at the provided branch, which is a subset of the provided ID. However, I was also able to reproduce this with the ID unaltered.

[lexnv]

We have two different backends.

The legacy backend, which makes calls to the legacy RPC methods (ie chain_subscribeFinalizedHeads).
And an unstable backend, which makes calls to the RPC V2 methods (ie chainHead_unstable_follow). For more info, check https://github.com/paritytech/json-rpc-interface-spec. The backend is called unstable to reflect that chainHead and transaction APIs are in flux.

When we construct a light-client instance in subxt, the default backend is the legacy one.
However, the RPC V2 was introduced to make the integration with light-clients better.

I've encountered a similar issue with the legacy RPCs in combination with smoldot at: smol-dot/smoldot#1638.

Next steps here:

• I'll raise an issue tomorrow in smoldot with some extra details
• I'll change the default backend for the smoldot to the unstable backend in a future PR (with option to fallback/use the legacy backend -- that will allow us to do extensive testing in the near future on the unstable-lightclient)

Meanwhile, I've got a patch to unblock you Oliver: https://github.com/paritytech/subxt/compare/lexnv/oty-light-client-iter-duplicate-unstable?expand=1 (uses the unstable backend for light-clients), let me know if this helps 🙏

Thanks again for raising this!

[ggwpez]

Meanwhile, I've got a patch to unblock you Oliver: https://github.com/paritytech/subxt/compare/lexnv/oty-light-client-iter-duplicate-unstable?expand=1 (uses the unstable backend for light-clients), let me know if this helps

Thanks! I was able to use LightClient now instead of RPC for this project https://github.com/ggwpez/fellowship-compliance, and actually it is faster than RPC 🤯

[jsdw]

I'll change the default backend for the smoldot to the unstable backend in a future PR (with option to fallback/use the legacy backend -- that will allow us to do extensive testing in the near future on the unstable-lightclient)

Rather than doing that right now (remember, doing so would requite that users take the UnstableBackendDriver and run it in a task, which is fine but because it's not what "normal" subxt does yet, I'd rather wait till we have a "standard" approach for this), I think a great option would be to allow the LightClientBuilder to be built from an arbitrary Backend in the same way that the OnlineClient is; this would allow people (and us) to experiment with both backends more easily without committing to any particular higher level interface just yet :)

[jsdw]

Just FYI, I merged a PR recently to revamp the light client interface which should help work around this: https://github.com/paritytech/subxt/blob/master/subxt/examples/light_client_basic.rs

The idea now is that you construct an RpcClient implementaiton which is the light client, and then pass that to Subxt to initialise it. In the example above, we just initialise an OnlineClient directly (which defaults to using the LegacyBackend, ie the old RPC methods, under the hood), but now you can also instantiate a backend to use the new (but still unstable!) RPCs with something like so:

#[tokio::main]
async fn main() -> Result<(), Box<dyn std::error::Error>> {
    // The lightclient logs are informative:
    tracing_subscriber::fmt::init();

    // Instantiate a light client with the Polkadot relay chain,
    // and connect it to Asset Hub, too.
    let (lightclient, polkadot_rpc) = LightClient::relay_chain(POLKADOT_SPEC)?;
    let asset_hub_rpc = lightclient.parachain(ASSET_HUB_SPEC)?;

    let (unstable_backend, driver) = subxt::backend::UnstableBackend::builder().build(asset_hub_rpc);

    // Unstable backend needs manually driving at the moment (this way, people
    // can choose which async runtime to use):
    tokio::spawn!(async move {
        while let Some(val) = driver.next().await {
            if let Err(e) = val {
                // Something went wrong driving unstable backend.
                break;
            }
        }
    })

    // Create Subxt clients from this unstable backend (ie using new RPCs).
    let polkadot_api = OnlineClient::<PolkadotConfig>::from_backend(unstable_backend).await?;
}

[jsdw]

The issue here was the Smoldot state_getKeysPaged API being different from the Substrate one (ie returning the start_key provided). I pushed a small workaround in Subxt in #1534 which will remove such a key if it exists, but hopefully the issue will be fixed upstream via the above Smoldot issue.