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

WEB3-208: Add IRiscZeroSelectable interface #313

Merged
merged 13 commits into from
Nov 26, 2024
Merged

Conversation

capossele
Copy link
Contributor

@capossele capossele commented Oct 29, 2024

Thi PR introduces a new interface called IRiscZeroSelectable that leverages the already existing SELECTOR constant of the RiscZeroGroth16Verifier (and more in general of any base implementation) to abstract from the base implementation when requiring the value of its selector (e.g., during deployment).

It also adds a Makefile to quickly deploy a local devnet and fixes some printing to create a proper TOML block e.g.,

  [[chains.anvil.verifiers]]
  version = "1.2.0-alpha.1"
  selector = "0x50bd1769"
  verifier = "0x9fe46736679d2d9a65f0992f2272de9f3c7fa6e0"
  estop = "0xcf7ed3acca5a467e9e704c703e8d87f634fb0fc9"

@github-actions github-actions bot changed the title Add IRiscZeroSelectable interface WEB3-208: Add IRiscZeroSelectable interface Oct 29, 2024
@capossele capossele marked this pull request as ready for review October 29, 2024 14:23
@capossele capossele requested a review from a team as a code owner October 29, 2024 14:23
Comment on lines 141 to 143
require(verifier.SELECTOR() == hex"50bd1769");
IRiscZeroSelectable selectable = IRiscZeroSelectable(verifier);
require(selectable.SELECTOR() == hex"50bd1769");
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this change should be required, right? Because the SELECTOR function exists on RiscZeroGroth16Verifier

@nategraf nategraf enabled auto-merge (squash) November 26, 2024 16:54
@nategraf
Copy link
Contributor

I like the contents of this PR, except the Makefile, so I removed that file in 44c372b and am going to merge this PR.

My understanding is that the problem that the makefile is solving is that we need a good way to create a deployment that includes the router and associated verifiers. Instead of a Makefile, I think we should extend the functionality we have in RiscZeroCheats.sol now, and provide this as a way to create a devnet programmatically using either forge script with a script provided in this repo, or importing and using that that deploy function in a larger script, as we might be in the Boundless repo.

/// @notice Deploy either a test or fully verifying `RiscZeroGroth16Verifier` depending on `devMode()`.
function deployRiscZeroVerifier() internal returns (IRiscZeroVerifier) {
if (devMode()) {
// NOTE: Using a fixed selector of 0 for the selector of the mock verifier.
IRiscZeroVerifier verifier = new RiscZeroMockVerifier(bytes4(0));
console2.log("Deployed RiscZeroMockVerifier to", address(verifier));
return verifier;
} else {
IRiscZeroVerifier verifier = new RiscZeroGroth16Verifier(ControlID.CONTROL_ROOT, ControlID.BN254_CONTROL_ID);
console2.log("Deployed RiscZeroGroth16Verifier to", address(verifier));
return verifier;
}
}

@nategraf nategraf merged commit f56c089 into main Nov 26, 2024
11 checks passed
@nategraf nategraf deleted the angelo/selectable branch November 26, 2024 17:07
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