Skip to content

Conversation

@invocamanman
Copy link
Contributor

Description

Include a test folder where a full bridge proof using Sp1 is done

PR Checklist:

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • [] I have made corresponding changes to the documentation
  • I have added or updated tests that comprehensively prove my change is effective or that my feature works

@invocamanman invocamanman requested a review from a team as a code owner June 18, 2025 23:52
@invocamanman invocamanman requested review from Freyskeyd and atanmarko and removed request for a team June 18, 2025 23:52
Comment on lines +6 to +15
pub fn main() {
// Read the bridge constraints input from stdin
let bridge_input: BridgeConstraintsInput = sp1_zkvm::io::read::<BridgeConstraintsInput>();

// Verify the bridge constraints - this will panic if verification fails
bridge_input.verify().unwrap();

// Commit the result to indicate successful verification
sp1_zkvm::io::commit(&true);
} No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

or instead of making a new program which could potentially diverge from what we do in the aggchain proof program, what about using our existing aggchain proof program in optimistic mode instead, which would then vastly focus on the bridge constraints?

Comment on lines +8 to +25
#[derive(Parser)]
#[command(name = "bridge-constraints-sp1-script")]
#[command(about = "Bridge Constraints SP1 Proof Test")]
struct Args {
#[arg(long, help = "Generate and verify cryptographic proof")]
prove: bool,
}

#[tokio::main]
async fn main() -> Result<()> {
// Load environment variables
dotenv::dotenv().ok();

// Parse command line arguments
let args = Args::parse();

println!("🌉 Bridge Constraints SP1 Proof Test");
println!("=====================================");
Copy link
Contributor

Choose a reason for hiding this comment

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

i think could be a simple test instead that we could even run in the ci rather than a dedicated binary no?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I think they should all be CI tests, would be easier for regression testing. No reason to have separate binaries here.

Comment on lines +75 to +77
println!("Executing SP1 program...");
let (mut output, report) = client.execute(&elf, &stdin).run()?;
println!("✓ Executed ({} cycles)", report.total_instruction_count());
Copy link
Contributor

Choose a reason for hiding this comment

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

and we would just need this in the ci test then, we wont need to generate a proof given that if the sp1 execute succeed then no reason for the actual proof generation to fail

Comment on lines +677 to +687
// Remove comment lines from the JSON string before parsing
let json_clean: String = CUSTOM_JSON
.lines()
.filter(|line| !line.trim_start().starts_with("//"))
.collect::<Vec<_>>()
.join("\n");

// Parse the JSON into alloy_genesis::Genesis first, then extract the config
let genesis_parsed: alloy::genesis::Genesis =
serde_json::from_str(&json_clean).expect("Failed to parse genesis JSON");

Copy link
Member

Choose a reason for hiding this comment

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

Could the function aggchain_proof_contracts::config::parse_evm_sketch_genesis be used here? It could be fixed/updated if needed.

"isthmusTime": 0
}
}

No newline at end of file
Copy link
Member

Choose a reason for hiding this comment

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

Proper EOF :)

aggchain-proof-core = { path = "../..", default-features = false }

[build-dependencies]
sp1-cli = "=5.0.0" No newline at end of file
Copy link
Member

Choose a reason for hiding this comment

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

Proper EOF :)

Comment on lines +10 to +15
[dependencies]
sp1-zkvm = "=5.0.0"
aggchain-proof-core = { path = "../..", default-features = false }

[build-dependencies]
sp1-cli = "=5.0.0" No newline at end of file
Copy link
Member

Choose a reason for hiding this comment

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

Use the workspace sp1 dependencies.

Comment on lines +7 to +18
[build-dependencies]
sp1-build = "5.0.5"
sp1-cli = "=5.0.0"

[dependencies]
aggchain-proof-core = { path = "../.." }
sp1-sdk = "=5.0.3"
serde_json = "1.0"
tokio = { version = "1.40", features = ["full"] }
anyhow = "1.0"
dotenv = "0.15.0"
clap = { version = "4.0", features = ["derive"] } No newline at end of file
Copy link
Member

Choose a reason for hiding this comment

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

Use the workspace versions. Otherwise, we will diverge.

Comment on lines +8 to +25
#[derive(Parser)]
#[command(name = "bridge-constraints-sp1-script")]
#[command(about = "Bridge Constraints SP1 Proof Test")]
struct Args {
#[arg(long, help = "Generate and verify cryptographic proof")]
prove: bool,
}

#[tokio::main]
async fn main() -> Result<()> {
// Load environment variables
dotenv::dotenv().ok();

// Parse command line arguments
let args = Args::parse();

println!("🌉 Bridge Constraints SP1 Proof Test");
println!("=====================================");
Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I think they should all be CI tests, would be easier for regression testing. No reason to have separate binaries here.

Base automatically changed from fix/upgrade-sp1-cc to main June 24, 2025 10:12
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.

4 participants