-
Notifications
You must be signed in to change notification settings - Fork 21
zkASM RLPAUTH #853
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
base: master
Are you sure you want to change the base?
zkASM RLPAUTH #853
Changes from 6 commits
fc3cbe0
05eb751
1f3eb0d
833116b
9e3a938
64270f9
00b2a0e
3e0db94
a96f719
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| include "../constants/evm.zkasm" | ||
|
|
||
| ;; https://eips.ethereum.org/EIPS/eip-7702 | ||
|
|
||
| ;; TODO: do we have an shared notation for constants, function arguments...? | ||
| ;; TODO: here we can directly have secp256k1n_divided_by_two | ||
| const SECP256K1N = 0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffefffffc2f | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Wrong cryptographic constant used for signature validationThe There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Wrong secp256k1 constant uses field prime instead of curve orderThe |
||
| const MAGIC = 0x05 | ||
|
|
||
| fn rlpauth(chain_id u256, nonce u64, address u160, y_parity u8, r u256, s u256) -> (authority u160, error u1) | ||
| { | ||
| ;; The following checks are enforced by the types above: | ||
| ;; assert auth.chain_id < 2**256 | ||
| ;; assert auth.nonce < 2**64 | ||
| ;; assert len(auth.address) == 20 | ||
| ;; assert auth.y_parity < 2**8 | ||
| ;; assert auth.r < 2**256 | ||
| ;; assert auth.s < 2**256 | ||
| step_1: | ||
| if chain_id == 0 goto step_3 | ||
| if chain_id == 1 goto step_3 ;; TODO: this should check current chain_id instead | ||
| fail | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Hardcoded chain ID validation enables cross-chain replayThe chain ID validation hardcodes acceptance of |
||
| ;; step_2 is implicitly succeseful due to nonce being declared as u64 | ||
| step_3: | ||
| ;; Divide SECP256K1N by 2 | ||
| var secp256k1n_divided_by_two u256 | ||
| var b u1 | ||
| secp256k1n_divided_by_two, b = SECP256K1N | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Missing division operation for SECP256K1N comparisonThe comment says "Divide SECP256K1N by 2" but the code There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Missing division operation for secp256k1n half calculationThe comment states "Divide SECP256K1N by 2" but the line There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Division by two not actually performedThe comment states "Divide SECP256K1N by 2" but the code |
||
| ;; Check that s <= SECP256K1N / 2 | ||
| var tmp u256 | ||
| b, tmp = s - secp256k1n_divided_by_two | ||
| if b == 1 goto failure | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Inverted logic for S malleability check comparisonThe check There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Inverted borrow check logic for S validationThe condition There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: S-value comparison logic is invertedThe s-value malleability check logic is inverted. The code |
||
| ;; | ||
| var rlp_res u256 | ||
| rlp_res = compute_rlp(chain_id, address, nonce) | ||
| ;; | ||
| var keccak_input u264 | ||
| keccak_input = compute_concat_magic_and_rlp(MAGIC, rlp_res) | ||
| ;; | ||
| var msg u256 | ||
| msg = keccak(keccak_input) | ||
| authority = ecrecover(msg, y_parity, r, s) | ||
| step_4: | ||
| ;; Add authority to accessed_addresses, as defined in EIP-2929. Does it have any effect here? | ||
| step_5: | ||
| ;; if authority is emtpy goto step_6 | ||
| ;; if authority is already delegated goto step_6 | ||
| ;; fail | ||
| step_6: | ||
| ;; if authority.nonce == nonce goto step_7 | ||
| ;; fail | ||
| step_7: | ||
| ;; Add PER_EMPTY_ACCOUNT_COST - PER_AUTH_BASE_COST gas to the global refund counter if authority is not empty. Does it have any effect here? | ||
| step_8: | ||
| ;; if address == 0x0000000000000000000000000000000000000000 goto "authority.code_hash = 0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470" | ||
| ;; authority.code = 0xef0100 || address | ||
| step_9: | ||
| ;; authority.nonce = authority.nonce + 1 | ||
| exit: | ||
| ;; dummy values | ||
| authority = 0 | ||
| error = 0 | ||
| return | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Computed authority overwritten before returningControl flow falls through to |
||
| failure: | ||
| fail | ||
| } | ||
|
|
||
| ;; temporary dummy functions | ||
| ;; TODO: determine the exact size of RLP | ||
| fn compute_rlp(chain_id u256, address u160, nonce u64) -> (rlp_res u256) { | ||
| rlp_res = 0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffefffffc2f | ||
| return | ||
| } | ||
|
|
||
| fn compute_concat_magic_and_rlp(magic u8, rlp u256) -> (concat_res u264) { | ||
| ;; TODO: how to concat? | ||
| ;; concat_res = magic, rlp? | ||
| concat_res = 0x05fffffffffffffffffffffffffffffffffffffffffffffffffffffffefffffc2f | ||
| return | ||
| } | ||
|
|
||
| fn keccak(keccak_input u264) -> (keccak_output u256) { | ||
| keccak_output = 0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffefffffc2f | ||
| return | ||
| } | ||
|
|
||
| fn ecrecover(msg u256, y_parity u8, r u256, s u256) -> (ecrecover_result u160) { | ||
| ecrecover_result = 0xfffffffffffffffffffffffffffffffefffffc2f | ||
| return | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Wrong constant used: field prime instead of curve order
The constant
SECP256K1Nis set to0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffefffffc2fwhich is the secp256k1 field prime (p), not the curve order (n). The correct curve order for signature malleability checks is0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364141. Using the field prime instead of the curve order makes the s-malleability check ineffective as it allows larger S values than permitted by EIP-7702.