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

Investigate zombienet-polkadot-functional-0003-beefy-and-mmr test failure #4309

Closed
serban300 opened this issue Apr 26, 2024 · 2 comments · Fixed by #5417
Closed

Investigate zombienet-polkadot-functional-0003-beefy-and-mmr test failure #4309

serban300 opened this issue Apr 26, 2024 · 2 comments · Fixed by #5417
Assignees
Labels
I2-bug The node fails to follow expected behavior. T10-tests This PR/Issue is related to tests.

Comments

@serban300
Copy link
Contributor

https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6059252

@serban300 serban300 added I2-bug The node fails to follow expected behavior. T10-tests This PR/Issue is related to tests. labels Apr 26, 2024
@serban300 serban300 self-assigned this Apr 26, 2024
@serban300
Copy link
Contributor Author

Ran it a lot of times locally and it didn't reproduce. Also doesn't seem to reproduce in the CI. I will close the issue for the moment.

@serban300
Copy link
Contributor Author

I have an idea about what might have caused the failure:

Copying the error here in case the zombienet log is deleted

2024-04-25 13:33:18        API/INIT: rococo/1010000: Not decorating unknown runtime apis: 0x6ff52ee858e6c5bd/1, 0xfbc577b9d747efd6/1
2024-04-25 13:33:18        RPC-CORE: verifyProofStateless(root: MmrHash, proof: MmrLeafBatchProof): bool:: 8013: Invalid proof: Error::Verify
 Error running script: ./0003-mmr-generate-and-verify-proof.js 	 8013: Invalid proof: Error::Verify

The test code:

async function run(nodeName, networkInfo, nodeNames) {
  const apis = await common.getApis(networkInfo, nodeNames);

  const proof = await apis[nodeName].rpc.mmr.generateProof([1, 9, 20]);

  const root = await apis[nodeName].rpc.mmr.root()

  const proofVerifications = await Promise.all(
    Object.values(apis).map(async (api) => {
      return api.rpc.mmr.verifyProof(proof);
    })
  );

  const proofVerificationsStateless = await Promise.all(
    Object.values(apis).map(async (api) => {
      return api.rpc.mmr.verifyProofStateless(root, proof);
    })
  );

  // check that all nodes accepted the proof
  return proofVerifications.every((proofVerification) => proofVerification) && proofVerificationsStateless.every((proofVerification) => proofVerification)
}

If a new block is generated between these 2 lines:

  const proof = await apis[nodeName].rpc.mmr.generateProof([1, 9, 20]);

  const root = await apis[nodeName].rpc.mmr.root()

we will try to verify a proof for the previous block with the mmr root at the current block. Which will fail.

@serban300 serban300 reopened this May 8, 2024
github-merge-queue bot pushed a commit that referenced this issue Aug 20, 2024
Fixes #4309

If a new block is generated between these 2 lines:

```
  const proof = await apis[nodeName].rpc.mmr.generateProof([1, 9, 20]);

  const root = await apis[nodeName].rpc.mmr.root()
```

we will try to verify a proof for the previous block with the mmr root
at the current block. Which will fail.

So we generate the proof and get the mmr root at block 21 for
consistency.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I2-bug The node fails to follow expected behavior. T10-tests This PR/Issue is related to tests.
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

1 participant