WEB3-548: Add RiscZeroVerifierFallbackRouter #700
Conversation
github-actions
bot
changed the title
nategraf
left a comment
nategraf
left a comment
There was a problem hiding this comment.
Looks good. I do have a few comments.
On the name "fallback" doesn't quite give the right idea, in that the upstream router will be used most of the time. The best name I could think of would be something like OverrideRouter where the RISC Zero router would be the base or parent.
| // NOTE: If there ever _is_ a reason to remove a selector that has never been set, the owner | ||
| // can call addVerifier with the tombstone address. | ||
| if (verifiers[selector] == UNSET) { | ||
| revert SelectorUnknown({selector: selector}); | ||
| } |
There was a problem hiding this comment.
When might want to actually change this now to either check if the upstream router has a a value set, or simply always set a tombstone. Reason being that if you may actually want to remove a verifier that's set on the upstream, this function would be the obvious way to do that.
| /// @dev Extends RiscZeroVerifierRouter to add fallback behavior. | ||
| contract RiscZeroVerifierFallbackRouter is RiscZeroVerifierRouter { | ||
| /// @notice The canonical RISC Zero verifier router used as fallback. | ||
| IRiscZeroVerifier public immutable fallbackRouter; |
There was a problem hiding this comment.
To function properly, this needs to be RiscZeroVerifierRouter. Instead of casting it in addVerifier, we could simply change the type here.
| IRiscZeroVerifier public immutable fallbackRouter; | |
| RiscZeroVerifierRouter public immutable fallbackRouter; |
| // If the verifier is unset, fall back to the canonical router. | ||
| if (verifier == UNSET) { | ||
| return fallbackRouter; | ||
| } |
There was a problem hiding this comment.
This means that boundlessRouter.getVerifier(0x0f00) will return the upstream router. This is a bit odd. It would be better to call getVerifier on the upstream router instead. If the goal here is to avoid two calls to the upstream rounter on the verify path, you might override the verify and verifyIntegrity methods as well to do so.
This PR proposes the introduction of a new smart contract,
RiscZeroVerifierFallbackRouter, which extends the functionality of the existingRiscZeroVerifierRouterby incorporating a fallback mechanism to a canonical verifier router. This allows for the secure addition of potentially higher-risk verifiers (e.g., third-party implementations) while reusing verifiers from a trusted canonical source.