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

ForbiddenSlotType when syncing Kusama from genesis #738

Closed
DragonDev1906 opened this issue Jun 14, 2023 · 5 comments · Fixed by #739
Closed

ForbiddenSlotType when syncing Kusama from genesis #738

DragonDev1906 opened this issue Jun 14, 2023 · 5 comments · Fixed by #739

Comments

@DragonDev1906
Copy link
Contributor

Hello,
I'm trying to sync Kusama from the genesis block using NonFinalizedTree<T> (see the code below) but I'm getting the following error:

VerificationFailed(BabeVerification(ForbiddenSlotType))

It works when trying to sync from genesis on Polkadot (same code, just replacing chain specification, hash and header)

Chain specification used: https://github.com/paritytech/polkadot/blob/master/node/service/chain-specs/kusama.json
Block 1 1: I've requested this block through the JsonRPC api, json decoded and scale encoded it. Since the hash calculated from the scale encoded header still matches this should be correct.

  • According to the BabeApi_configuration runtime call (executed via polkadot.js.org and when debugging the result of the wasm call in smoldot), the SecondarySlot 2 is set to PrimaryAndSecondaryVRFSlots for both Polkadot and Kusama.
  • According to src/verify/babe.rs 3, smoldot uses the PreDigest of a block to determine if the slot type is allowed.

What I do not understand is why the PreDigest of Kusama block 1 1 is apparently part of the canonical chain but is not allowed to be the first block on said chain according to smoldot and what I understood from the BabeApi_configuration runtime call.

Kusama block 1 PreDigest:

02 // SecondaryPlain
01000000 // 32-bit authority set index of block author
ef55a50f00000000 // 64-bit slot number

Polkadot block 1 PreDigest:

01 // Primary
00000000 // authority set index of block author
93decc0f00000000 // 64-bit slot number
362ed8d6055645487fe42e9c8640be651f70a3a2a03658046b2b43f021665704501af9b1ca6e974c257e3d26609b5f68b5b0a1da53f7f252bbe5d94948c39705c98ffa4b869dd44ac29528e3723d619cc7edf1d3f7b7a57a957f6a7e9bdb270a // VTF output and proof

(decoded according to 4)

Smoldot first checks for the parent hash 5 before checking Babe related information 6, so the parent_hash must match the genesis block in the chain configuration. So if we have a wrong/bad chain specification smoldot probably complain somewhere that the wasm code did not match the code in the genesis block, otherwise I don't know how we'd get into this situation unless this header is invalid and polkadot.js.org gives me bad Information.

So here is my conclusion so far on what the reason could be (I hope you can confirm or deny some of these):

  • I might have a bad/wrong chain specification, if that is the case, shouldn't smoldot complain somewhere earlier because the parent hash does not match the genesis block or the genesis block contains a different wasm code than used during configuration. In that case, could someone point me to a good/correct chain specification for syncing from the genesis block please?
  • I might have an invalid block that is not part of the canonical Kusama chain. This could be a long range attack, but I don't see a reason that the node would provide bad data and I've tried multiple providers in polkadot.js.org. I also find it more likely that there is a bug in my code/setup or in smoldot than multiple nodes providing bad blocks. I cannot easily check if this block was finalized according to Grandpa, but even then it should still be valid according to Babe (unless, as said earlier) both tested providers have//return an invalid block.
  • There might have been a breaking change somewhere in Kusama that breaks something really fundamental, like changing the encoding of PreDigest (or smoldot just does not support syncing from genesis, but it did work for Polkadot). Also something I find incredibly unlikely.
  • The kusama chain is somehow broken: Also unlikely
  • The wasm code was not executed correctly or the wrong code was used. Also seems somewhat unlikely, since I got the same PrimaryAndSecondaryVRFSlots when using polkadot.js.org (unless that just executes smoldot code in the background, which could be the case).
  • The encoded header is not encoded correctly: Unlikely because the hash still matches and this wouldn't answer why the PreDigest doesn't seem to match the allowed types after the genesis block.
  • There might be a bug in smoldot (hence this issue)
  • I'm stupid and/or blind and I'm missing something really obvious

I hope someone can help me with that. While I could sync from a later checkpoint (and will probably end up doing so anyways), I'd still like to get a sync from genesis to work.

Thank you in advance (and sorry for the long text).

PS: Yes, I know about the possibility of long-range attacks.

Minimal example

use std::time::{SystemTime, UNIX_EPOCH};

use smoldot::{
    chain::blocks_tree::{Config, HeaderVerifySuccess, NonFinalizedTree},
    chain_spec::ChainSpec,
    header::hash_from_scale_encoded_header,
};

#[tokio::main]
async fn main() {
    let chain_spec = include_str!("kusama.json");
    let chain_spec = ChainSpec::from_json_bytes(chain_spec).unwrap();
    let chain_information = chain_spec.to_chain_information().unwrap().0;

    let block_number_bytes = chain_spec.block_number_bytes().into();

    let config = Config {
        chain_information,
        block_number_bytes,
        blocks_capacity: 1024,
        allow_unknown_consensus_engines: true,
    };
    let mut tree = NonFinalizedTree::<()>::new(config);

    // Number: 1
    // Hash: 0xcd9b8e2fc2f57c4570a86319b005832080e0c478ab41ae5d44e23705872f5ad3
    let scale_encoded_header = hex_literal::hex!("b0a8d493285c2df73290dfb7e61f870f17b41801197a149ca93654499ea3dafe04fabb0c6e92d29e8bb2167f3c6fb0ddeb956a4278a3cf853661af74a076fc9cb7a35fb7f7616f5c979d48222b3d2fa7cb2331ef73954726714d91ca945cc34fd80c0642414245340201000000ef55a50f00000000044241424549040118ca239392960473fe1bc65f94ee27d890a49c1b200c006ff5dcc525330ecc16770100000000000000b46f01874ce7abbb5220e8fd89bede0adad14c73039d91e28e881823433e723f0100000000000000d684d9176d6eb69887540c9a89fa6097adea82fc4b0ff26d1062b488f352e179010000000000000068195a71bdde49117a616424bdc60a1733e96acb1da5aeab5d268cf2a572e94101000000000000001a0575ef4ae24bdfd31f4cb5bd61239ae67c12d4e64ae51ac756044aa6ad8200010000000000000018168f2aad0081a25728961ee00627cfe35e39833c805016632bf7c14da5800901000000000000000000000000000000000000000000000000000000000000000000000000000000054241424501014625284883e564bc1e4063f5ea2b49846cdddaa3761d04f543b698c1c3ee935c40d25b869247c36c6b8a8cbbd7bb2768f560ab7c276df3c62df357a7e3b1ec8d");
    let hash =
        hex_literal::hex!("cd9b8e2fc2f57c4570a86319b005832080e0c478ab41ae5d44e23705872f5ad3");

    assert_eq!(hash_from_scale_encoded_header(scale_encoded_header), hash);

    let success = tree
        .verify_header(
            scale_encoded_header.into(),
            SystemTime::now().duration_since(UNIX_EPOCH).unwrap(),
        )
        .expect("to succeed because that header came from a kusama node and hash matches");

    match success {
        HeaderVerifySuccess::Duplicate => unreachable!(),
        HeaderVerifySuccess::Insert { insert, .. } => insert.insert(()),
    }

    println!("Done");
}

Footnotes

  1. https://polkadot.js.org/apps/?rpc=wss%3A%2F%2Fksm-rpc.stakeworld.io#/explorer/query/0xcd9b8e2fc2f57c4570a86319b005832080e0c478ab41ae5d44e23705872f5ad3 2

  2. https://spec.polkadot.network/sect-block-production#defn-consensus-message-babe

  3. https://github.com/smol-dot/smoldot/blob/main/lib/src/verify/babe.rs#L239

  4. https://spec.polkadot.network/sect-block-production#defn-babe-header

  5. https://github.com/smol-dot/smoldot/blob/main/lib/src/chain/blocks_tree/verify.rs#L156

  6. https://github.com/smol-dot/smoldot/blob/main/lib/src/chain/blocks_tree/verify.rs#L246

@tomaka
Copy link
Contributor

tomaka commented Jun 14, 2023

Hi,

I would lean towards a bug in smoldot. While the code of smoldot is generally in good shape, the lack of unit tests means that there are probably many small mistakes. Given that plain slots aren't used anymore on any chain, it's likely that this went undetected.

According to the BabeApi_configuration runtime call (executed via polkadot.js.org and when debugging the result of the wasm call in smoldot), the SecondarySlot 2 is set to PrimaryAndSecondaryVRFSlots for both Polkadot and Kusama

Note that you have to do the runtime call against block 0, as this configuration has changed over time. PolkadotJS unfortunately doesn't give you the possibility to choose which block to execute the call against.

From the top of my head (I haven't checked), the Kusama genesis block should be configured as PrimaryAndSecondaryPlainSlots (and not VRF slots). I'm pretty confident that this is the case, because the concept of plain slots comes from an earlier draft of Babe. VRF slots were later introduced and replaced plain slots.

@tomaka
Copy link
Contributor

tomaka commented Jun 14, 2023

My suspicion would be that this is wrong:

header::BabeAllowedSlots::PrimaryAndSecondaryVrfSlots

It is the code that decodes the BABE configuration when the version of the BABE API was still version 1, a long time ago.

@tomaka
Copy link
Contributor

tomaka commented Jun 14, 2023

I think it is indeed wrong.

The Substrate code that turns a BabeConfigurationV1 into a BabeConfiguration (the recent one) converts secondary_slots: true to allowed_slots: PrimaryAndSecondaryPlainSlots.
https://github.com/paritytech/substrate/blob/689da495a0c0c0c2466fe90a9ea187ce56760f2d/primitives/consensus/babe/src/lib.rs#L173

@DragonDev1906
Copy link
Contributor Author

PolkadotJS unfortunately doesn't give you the possibility to choose which block to execute the call against.

True, I'd hoped that this configuration would be something that doesn't change in the runtime, given that it is supposed to only be executed at the genesis block. But this of course also means that nothing breaks if it is changed afterwards since all nodes use the genesis runtime (though that makes this call in PolkadotJS rather useless).

@tomaka
Copy link
Contributor

tomaka commented Jun 14, 2023

Thanks for finding and reporting this!

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 a pull request may close this issue.

2 participants