Skip to content

add chain_id to steel and sp1-cc host, client, and verifier#99

Merged
bxue-l2 merged 5 commits into
masterfrom
fix--pass-l1-chain-id-to-canoe-provider-and-verifier
May 19, 2025
Merged

add chain_id to steel and sp1-cc host, client, and verifier#99
bxue-l2 merged 5 commits into
masterfrom
fix--pass-l1-chain-id-to-canoe-provider-and-verifier

Conversation

@bxue-l2

@bxue-l2 bxue-l2 commented May 16, 2025

Copy link
Copy Markdown
Collaborator

This PR fixes a security issue that chain_id must be attached to canoe to constrain the evm execution rule

  • l1_chain_id is added to canoe host/apps to generate right evm state
  • l1_chain_id is committed by the client/methods to bind the evm chain_id with execution
  • l1_chain_id is copied from bootInfo which comes from oracle, and presumably verified or to be verified by kona
  • the canoe verifier makes sense the journal committed by the canoe client/methods matches the chain_id from the bootInfo

@bxue-l2 bxue-l2 requested a review from samlaf May 16, 2025 05:15
Comment thread canoe/bindings/src/lib.rs
Comment thread canoe/provider/src/lib.rs Outdated
Comment thread canoe/sp1-cc/client/src/main.rs
Comment thread canoe/sp1-cc/host/src/lib.rs Outdated
Comment thread canoe/sp1-cc/host/src/lib.rs
Comment on lines +50 to +54

// sp1-cc currently has limitation on supporting custom chain_id without supplying genesis json
// overwriting cert_validity chain_id to be 1, which is the default mainnet chain_id used by
// sp1-cc host when chain spec is not specified
cert_validity.l1_chain_id = 1;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't you modify this before passing it in to validate_cert_receipt? I really don't like seeing mut on inputs

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that will touch the code living in the common path shared between steel and sp1-cc.

For secure integration, chain_id is derived from the trusted BootInfo, so you can't change it on the sp1-cc host side.

We will need to revisit this code anyway in the future to test sp1-cc proof verification within zkvn

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are you linking random commit links that aren't related to the convo here lol?
Not following your argument "so you can't change it on the sp1-cc host side.". Just create a new struct before calling this function? Functions that do verification should be pure. Patrick keeps repeating this as well for our contacts. verification should be pure.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The client cannot blindly trust the host, see the updated README

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread crates/proof/src/cert_validity.rs Outdated
Comment thread crates/zkvm-verification/src/lib.rs Outdated
@bxue-l2 bxue-l2 merged commit 056ee98 into master May 19, 2025
8 checks passed
@bxue-l2 bxue-l2 deleted the fix--pass-l1-chain-id-to-canoe-provider-and-verifier branch May 19, 2025 21:16
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 this pull request may close these issues.

2 participants