feat(v1.0.0): merkle #393
feat(v1.0.0): merkle #393
Conversation
* feat: simplify multichain nonce design * chore: readd merkle verification prefix * chore: undo blank line addns * chore: lint * chore: redundant multichain bool * fix: lint * .
* feat: simplify multichain nonce design * chore: readd merkle verification prefix * chore: undo blank line addns * chore: lint * . * ~50 failing tests down to 5 * down to 1 failing test * fixed failing test * chore: remove console logs and bench * . * Update test/Base.t.sol * Update src/Orchestrator.sol * Update test/utils/mocks/MockPayerWithSignatureOptimized.sol * chore: final cleanup, rebench * Update src/Orchestrator.sol * chore: bump contract versions due to bytecode changes - Contracts updated: IthacaAccount,Orchestrator,SimpleFunder,Simulator * chore: cleanup * rebase * fix --------- Co-authored-by: GitHub Action <[email protected]>
…ated: Orchestrator,SimpleFunder,Simulator
|
🤖 Bytecode changes detected! EIP-712 domain versions have been automatically updated for: Orchestrator,SimpleFunder,Simulator |
| internal | ||
| view | ||
| returns (bool isValid, bytes32 keyHash) | ||
| { |
There was a problem hiding this comment.
continuing convo from this: https://github.com/ithacaxyz/account/pull/388/files#r2387828490
I think the outer one is entirely ignored, so theres no security issue
return _verifyMerkleSig(digest, signature) -> this overrides the keyHash = ... parsed in the top level call frame
On prehash - IIUC there should only be 1 prehash that's set, in either the outer or inner wrapper. There's a difference between the 2 since the digest has to be in the merkle tree, so which one we want to set depends whether the merkle leaf is sha-ed or not. I don't know which one would be shaed in practice, but the other one would probably be redundant
There was a problem hiding this comment.
Something that I just thought to check was whether we'll run into issues around second preimage attacks
I don't see anything thats obvious so far, bringing it up just in case im missing something
Applies PR #388 to v1.0.0 account.
With an additional breaking change, which removes the orchestrator's redundant
verifyMerkleSigfunction, and unifies all flows through the account's native merkle sig verification.Note: large diff is because of new foundry fmt changes, the main changes are in the IthacaAccount, Orchestrator, Account.t.sol, and Orchestrator.t.sol files