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

Duplicate storage iterator items using lightclient #1743

Open
paulormart opened this issue Sep 1, 2024 · 6 comments
Open

Duplicate storage iterator items using lightclient #1743

paulormart opened this issue Sep 1, 2024 · 6 comments

Comments

@paulormart
Copy link
Contributor

paulormart commented Sep 1, 2024

Hi there,

This issue is a follow up of #1722

Fetching storage items through an iterator returns duplicates when using lightclient, hanging the loop forever with the current version of subxt. The behaviour is improved using the latest smoldot dependencies but there are still duplicates.

This isue can be confirmed when running the example below:

#![allow(missing_docs)]
use subxt::{client::OnlineClient, lightclient::LightClient, PolkadotConfig};
use std::collections::BTreeMap;

// Generate an interface that we can use from the node's metadata.
#[subxt::subxt(runtime_metadata_path = "../artifacts/polkadot_metadata_full.scale")]
pub mod polkadot {}

const POLKADOT_SPEC: &str = include_str!("../../artifacts/demo_chain_specs/polkadot.json");

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

    let (_, polkadot_rpc) = LightClient::relay_chain(POLKADOT_SPEC)?;
    
    // Create Subxt clients from these Smoldot backed RPC clients.
    let api = OnlineClient::<PolkadotConfig>::from_rpc_client(polkadot_rpc).await?;

    let address = polkadot::storage()
        .bounties()
        .bounties_iter();

    let mut iter = api.storage().at_latest().await?.iter(address).await?;

    let mut data = BTreeMap::new();
    while let Some(Ok(storage)) = iter.next().await {
        let id = get_bounty_id_from_storage_key(storage.key_bytes);
        data.entry(id).and_modify(|n| *n += 1).or_insert(1);
        assert_eq!(data.len() as u32, data.iter().map(|(_, v)| v).sum::<u32>());
    }
    
    Ok(())
}

pub fn get_bounty_id_from_storage_key(key: Vec<u8>) -> u32 {
    let s = &key[key.len() - 4..];
    let v: [u8; 4] = s.try_into().expect("slice with incorrect length");
    u32::from_le_bytes(v)
}
@paulormart
Copy link
Contributor Author

Ok, as mentioned here #1453 (comment) using the unstable backend and also using the latest smoldot v0.18.0 updated in #1722 fixes the issue with duplicate storage entries, but doesn't work with the current smoldot v0.16.0 available in subxt v0.37. In this case, the following error is raised Error driving unstable backend: Rpc error: RPC error: RPC Error: {"code":-32601,"message":"The method does not exist / is not available."}.

Example tested:

#![allow(missing_docs)]
use std::collections::BTreeMap;
use futures::StreamExt;
use subxt::{
    backend::unstable::UnstableBackend, client::OnlineClient, lightclient::LightClient,
    PolkadotConfig,
};

// Generate an interface that we can use from the node's metadata.
#[subxt::subxt(runtime_metadata_path = "../artifacts/polkadot_metadata_full.scale")]
pub mod polkadot {}

const POLKADOT_SPEC: &str = include_str!("../../artifacts/demo_chain_specs/polkadot.json");

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

    let (_, polkadot_rpc) = LightClient::relay_chain(POLKADOT_SPEC)?;

    let (unstable_backend, mut driver) = UnstableBackend::builder().build(polkadot_rpc);

    tokio::spawn(async move {
        while let Some(val) = driver.next().await {
            if let Err(e) = val {
                // Something went wrong driving unstable backend.
                eprintln!("Error driving unstable backend: {e}");
                break;
            }
        }
    });

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

    // Create Subxt clients from these Smoldot backed RPC clients.
    // let api = OnlineClient::<PolkadotConfig>::from_rpc_client(polkadot_rpc).await?;

    let address = polkadot::storage().bounties().bounties_iter();

    let mut iter = api.storage().at_latest().await?.iter(address).await?;

    let mut data = BTreeMap::new();
    while let Some(Ok(storage)) = iter.next().await {
        let id = get_bounty_id_from_storage_key(storage.key_bytes);
        data.entry(id).and_modify(|n| *n += 1).or_insert(1);
        assert_eq!(data.len() as u32, data.iter().map(|(_, v)| v).sum::<u32>());
    }

    println!("{:?}", data);

    Ok(())
}

pub fn get_bounty_id_from_storage_key(key: Vec<u8>) -> u32 {
    let s = &key[key.len() - 4..];
    let v: [u8; 4] = s.try_into().expect("slice with incorrect length");
    u32::from_le_bytes(v)
}

@niklasad1
Copy link
Member

niklasad1 commented Sep 2, 2024

Ok, as mentioned here #1453 (comment) using the unstable backend and also using the latest smoldot v0.18.0 updated in #1722 fixes the issue with duplicate storage entries, but doesn't work with the current smoldot v0.16.0 available in subxt v0.37. In this case, the following error is raised Error driving unstable backend: Rpc error: RPC error: RPC Error: {"code":-32601,"message":"The method does not exist / is not available."}.

This is because the method chainHead_v1_follow is named chainHead_unstable_follow in smoldot v0.16. Probably subxt could be smarter and call rpc_methods to identify which APIs are exposed by the node. So, the only option is to downgrade subxt to an older client before chainHead was stabilized.

Extracted logs below:

2024-09-02T07:50:33.196501Z DEBUG json-rpc-polkadot: JSON-RPC => {"jsonrpc":"2.0","id":"1", "method":"chainHead_v1_follow","params":[true]}
2024-09-02T07:50:33.196553Z DEBUG json-rpc-polkadot: JSON-RPC <= {"jsonrpc":"2.0","id":"1","error":{"code":-32601,"message":"The method does not exist / is not available."}}
Error driving unstable backend: Rpc error: RPC error: RPC Error: 2024-09-02T07:50:33.196639Z DEBUG runtime-polkadot: Worker <= FailedDownload(blocks=[0xfe68…869a], error=StorageQuery(StorageQueryError { errors: [] }))
{"code":-32601,"message":"The method does not exist / is not available."}.

I reckon that we probably should stabilize the unstable backend and update smoldot to fix this annoyance.

EDIT: I created another issue for ☝️ I reckon that isn't related to duplicate storage items iter, see #1749

@jsdw
Copy link
Collaborator

jsdw commented Sep 2, 2024

I wonder whether this is a duplicate or otherwise related to #1453.

Here, older versions of Smoldot had a slightly different "legacy" API impl to Substrate nodes, leading to the dupe entry. A fix was pushed to Subxt a while ago to accomodate this though.

@jsdw
Copy link
Collaborator

jsdw commented Sep 3, 2024

I added some logging to the fetching logic in the reproducible example above and got this:

Getting keys via state_get_keys_paged, pagination key:
None

Response:
[..., 223, 205, 161, 94, 52, 125, 117, 129, 32, 0, 0, 0]
[..., 44, 25, 84, 221, 159, 83, 207, 140, 38, 0, 0, 0]
[..., 26, 135, 220, 230, 199, 76, 76, 75, 37, 0, 0, 0]
[..., 64, 27, 83, 75, 157, 178, 6, 157, 52, 0, 0, 0]
[..., 80, 86, 200, 52, 234, 218, 252, 96, 39, 0, 0, 0]
[..., 176, 167, 35, 181, 81, 88, 134, 247, 58, 0, 0, 0]
[..., 9, 140, 98, 168, 3, 250, 164, 204, 48, 0, 0, 0]
[..., 4, 197, 200, 180, 230, 223, 147, 109, 40, 0, 0, 0]
[..., 3, 129, 165, 252, 69, 122, 161, 92, 54, 0, 0, 0]
[..., 1, 207, 232, 191, 118, 186, 39, 240, 30, 0, 0, 0]
[..., 62, 31, 98, 149, 228, 158, 210, 73, 44, 0, 0, 0]
[..., 60, 201, 195, 83, 192, 115, 126, 211, 41, 0, 0, 0]
[..., 56, 163, 88, 145, 190, 247, 174, 97, 57, 0, 0, 0]
[..., 79, 255, 102, 183, 202, 99, 22, 40, 33, 0, 0, 0]
[..., 87, 204, 157, 39, 224, 160, 78, 248, 45, 0, 0, 0]
[..., 84, 152, 146, 212, 75, 173, 214, 175, 19, 0, 0, 0]
[..., 107, 237, 251, 209, 118, 23, 153, 53, 43, 0, 0, 0]
[..., 106, 90, 114, 245, 120, 124, 177, 160, 53, 0, 0, 0]
[..., 127, 175, 90, 238, 118, 182, 218, 148, 47, 0, 0, 0]
[..., 114, 78, 207, 196, 142, 235, 76, 76, 31, 0, 0, 0]
[..., 142, 35, 63, 151, 34, 211, 14, 191, 49, 0, 0, 0]
[..., 135, 53, 27, 25, 242, 38, 254, 170, 24, 0, 0, 0]
[..., 146, 111, 238, 114, 211, 113, 249, 11, 51, 0, 0, 0]
[..., 146, 111, 57, 101, 250, 244, 95, 15, 22, 0, 0, 0]
[..., 169, 236, 99, 206, 125, 54, 124, 87, 27, 0, 0, 0]
[..., 166, 178, 116, 37, 14, 103, 83, 240, 10, 0, 0, 0]
[..., 190, 147, 216, 164, 206, 119, 153, 192, 11, 0, 0, 0]
[..., 202, 240, 66, 122, 170, 21, 230, 225, 56, 0, 0, 0]
[..., 194, 217, 103, 92, 249, 93, 207, 26, 17, 0, 0, 0]
[..., 243, 99, 65, 191, 187, 10, 78, 146, 50, 0, 0, 0]
[..., 241, 252, 80, 125, 59, 251, 253, 63, 36, 0, 0, 0]
[..., 241, 11, 197, 47, 182, 215, 86, 215, 42, 0, 0, 0]
[..., 71, 231, 30, 139, 171, 141, 241, 107, 55, 0, 0, 0]
[..., 71, 211, 225, 20, 88, 189, 183, 210, 59, 0, 0, 0]

Getting keys via state_get_keys_paged, pagination key:
Some([..., 71, 211, 225, 20, 88, 189, 183, 210, 59, 0, 0, 0])

Response:
[..., 223, 205, 161, 94, 52, 125, 117, 129, 32, 0, 0, 0]
[..., 80, 86, 200, 52, 234, 218, 252, 96, 39, 0, 0, 0]
[..., 176, 167, 35, 181, 81, 88, 134, 247, 58, 0, 0, 0]
[..., 79, 255, 102, 183, 202, 99, 22, 40, 33, 0, 0, 0]
[..., 87, 204, 157, 39, 224, 160, 78, 248, 45, 0, 0, 0]
[..., 84, 152, 146, 212, 75, 173, 214, 175, 19, 0, 0, 0]
[..., 107, 237, 251, 209, 118, 23, 153, 53, 43, 0, 0, 0]
[..., 106, 90, 114, 245, 120, 124, 177, 160, 53, 0, 0, 0]
[..., 127, 175, 90, 238, 118, 182, 218, 148, 47, 0, 0, 0]
[..., 114, 78, 207, 196, 142, 235, 76, 76, 31, 0, 0, 0]
[..., 142, 35, 63, 151, 34, 211, 14, 191, 49, 0, 0, 0]
[..., 135, 53, 27, 25, 242, 38, 254, 170, 24, 0, 0, 0]
[..., 146, 111, 238, 114, 211, 113, 249, 11, 51, 0, 0, 0]
[..., 146, 111, 57, 101, 250, 244, 95, 15, 22, 0, 0, 0]
[..., 169, 236, 99, 206, 125, 54, 124, 87, 27, 0, 0, 0]
[..., 166, 178, 116, 37, 14, 103, 83, 240, 10, 0, 0, 0]
[..., 190, 147, 216, 164, 206, 119, 153, 192, 11, 0, 0, 0]
[..., 202, 240, 66, 122, 170, 21, 230, 225, 56, 0, 0, 0]
[..., 194, 217, 103, 92, 249, 93, 207, 26, 17, 0, 0, 0]
[..., 243, 99, 65, 191, 187, 10, 78, 146, 50, 0, 0, 0]
[..., 241, 252, 80, 125, 59, 251, 253, 63, 36, 0, 0, 0]
[..., 241, 11, 197, 47, 182, 215, 86, 215, 42, 0, 0, 0]
[..., 71, 231, 30, 139, 171, 141, 241, 107, 55, 0, 0, 0]
[..., 71, 211, 225, 20, 88, 189, 183, 210, 59, 0, 0, 0]

I elided the start of the keys because they are all identical. Here, we see the API using the correct "pagination start key" to retrieve the second batch, but ultimately the response back from Smoldot 0.16 is overlapping in many entries.

Subxt tried to avoid this sort of issue by ignorign the first key we get back if it's identical to the "pagination start key", but this fix is obviously not working out here.

The fix I think will have to be to use the currently unstable backend against Smoldot 0.18, as I can't see an obvious way to avoid this issue in Subxt offhand.

@niklasad1
Copy link
Member

We have updated smoldot v0.18 so closing this.

Feel free to re-open if something isn't working

@jsdw
Copy link
Collaborator

jsdw commented Oct 18, 2024

It's worth checking: is the issue fixed using the latest smoldot but the default (ie legacy) backend?

looking above I'm not sure if it would be!

@jsdw jsdw reopened this Oct 18, 2024
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

No branches or pull requests

3 participants