@lightclient + @anna-carroll reviews #7
Conversation
| @@ -368,6 +368,9 @@ pub fn static_call<H: Host, SPEC: Spec>(interpreter: &mut Interpreter<'_>, host: | |||
|
|
|||
| // EIP-3074 | |||
| pub fn auth<H: Host, SPEC: Spec>(interpreter: &mut Interpreter<'_>, host: &mut H) { | |||
| // The minimum amount of memory expansion (in bytes) required to hold the signature and commit. | |||
| const MINIMUM_EXPANSION: usize = 97; | |||
There was a problem hiding this comment.
I don't believe this is necessarily true. It's possible to specify a memory region less than 97 bytes and the eip says to treat the rest of the expected bytes as zeros.
| let mem_slice = interpreter | ||
| .shared_memory | ||
| .slice(signature_offset, signature_len); | ||
| .slice(signature_offset, MINIMUM_EXPANSION); |
There was a problem hiding this comment.
this might be an ignorant question, but why do we expand memory by the full pointer length if we are only always going to slice exactly 97 bytes?
There was a problem hiding this comment.
what happens if we slice MINIMUM_EXPANSION length if signature_len is less than MINIMUM_EXPANSION length? could we slice into "dirty" memory region on accident?
| // The spec states that the `AUTH` opcode returns 0 and unsets the authorized account | ||
| // if the signature is either invalid, or the recovered address does not match the | ||
| // authority. | ||
| interpreter.authorized = None; |
There was a problem hiding this comment.
super super nit, but the order of these operations is switched between the success and failure cases, which makes it ever so slightly less readable
Overview
Addresses some review comments from #1
ActiveAccountUnsetAuthCallerror ->UnauthorizedAuthCallAUTH; At a minimum, the opcode should expand memory tooffset + 97if it is not already allocated. More expansion can occur iflength>97and the memory is not already allocated.AUTH: "If the signature is instead invalid or the signer address does not equalauthority,authorizedis reset to an unset value."