Cache verifier callers #1331
Cache verifier callers #1331
Conversation
Signed-off-by: litt3 <102969658+litt3@users.noreply.github.com>
Signed-off-by: litt3 <102969658+litt3@users.noreply.github.com>
| // maps contract address to a ContractEigenDACertVerifierCaller object | ||
| verifierCallers sync.Map |
There was a problem hiding this comment.
should there be a size constraint imposed on this mapping? ie if we take the design route where proxy takes in arbitrary addresses then this could present a DoS vector
There was a problem hiding this comment.
if we take the design route where proxy takes in arbitrary addresses then this could present a DoS vector
Valid point, but I can't think of a reason we would pursue a design route where proxy accepts arbitrary addresses: the two cases I can imagine are either cert verifier address being managed by an EigenDACertVerifierRouter, or a hardcoded or config value in proxy for cases where the address isn't permitted to change.
Can you think of a reason we'd allow arbitrary definitions for this?
If we can't think of a potential reason here, I think it makes sense to not limit the size of the map: doing so is non-trivial in complexity and would present a cost in the standard mode of operation. Perhaps just a comment calling out the concern, so it's more likely to attract notice if it ever needs to be changed?
|
|
||
| existingCaller, valueExists := cv.verifierCallers.Load(certVerifierAddress) | ||
| if valueExists { | ||
| return existingCaller.(*verifierBindings.ContractEigenDACertVerifierCaller), nil |
There was a problem hiding this comment.
any value in instead safe casting the caster to the binding type? looking at this code it should be impossible but without an invariant enforcement this does present a hypothetical panic
There was a problem hiding this comment.
Good point, fixed in 74ec1b9e
Signed-off-by: litt3 <102969658+litt3@users.noreply.github.com>
Why are these changes needed?
Checks