-
Notifications
You must be signed in to change notification settings - Fork 17
feat: self-delegate multicall helper (#1997) #2029
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
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,166 @@ | ||
| import Compiler.Codegen | ||
| import Compiler.CompilationModel | ||
| import Compiler.CompilationModel.TrustSurface | ||
| import Compiler.Modules.Calls | ||
| import Compiler.Yul.PrettyPrint | ||
|
|
||
| namespace Compiler.Modules.CallsTest | ||
|
|
||
| open Compiler | ||
| open Compiler.CompilationModel | ||
|
|
||
| private def contains (haystack needle : String) : Bool := | ||
| let h := haystack.toList | ||
| let n := needle.toList | ||
| if n.isEmpty then true | ||
| else | ||
| let rec go : List Char → Bool | ||
| | [] => false | ||
| | c :: cs => | ||
| if (c :: cs).take n.length == n then true | ||
| else go cs | ||
| go h | ||
|
|
||
| private def expectTrue (label : String) (ok : Bool) : IO Unit := do | ||
| if !ok then | ||
| throw (IO.userError s!"✗ {label}") | ||
| IO.println s!"✓ {label}" | ||
|
|
||
| private def selectorsFor (spec : CompilationModel) : List Nat := | ||
| List.range (spec.functions.filter (fun fn => | ||
| !fn.isInternal && fn.name != "fallback" && fn.name != "receive")).length | ||
|
|
||
| private def expectCompileErrorContains (label : String) | ||
| (spec : CompilationModel) (needle : String) : IO Unit := do | ||
| match Compiler.CompilationModel.compile spec (selectorsFor spec) with | ||
| | .ok _ => throw (IO.userError s!"✗ {label}: expected compile error containing '{needle}'") | ||
| | .error err => | ||
| if !contains err needle then | ||
| throw (IO.userError s!"✗ {label}: expected error containing '{needle}', got '{err}'") | ||
| IO.println s!"✓ {label}" | ||
|
|
||
| private def expectCompileToYul (label : String) (spec : CompilationModel) : IO String := do | ||
| match Compiler.CompilationModel.compile spec (selectorsFor spec) with | ||
| | .ok ir => | ||
| IO.println s!"✓ {label}" | ||
| pure (Compiler.Yul.render (Compiler.emitYul ir)) | ||
| | .error err => | ||
| throw (IO.userError s!"✗ {label}: compile failed: {err}") | ||
|
|
||
| private def selfDelegateMulticallBytesSmokeSpec : CompilationModel := { | ||
| name := "SelfDelegateMulticallBytesSmoke" | ||
| fields := [] | ||
| «constructor» := none | ||
| functions := [ | ||
| { name := "multicall" | ||
| params := [ | ||
| { name := "calls", ty := ParamType.array ParamType.bytes } | ||
| ] | ||
| returnType := none | ||
| body := [ | ||
| Compiler.Modules.Calls.selfDelegateMulticallBytes "calls", | ||
| Stmt.stop | ||
| ] | ||
| } | ||
| ] | ||
| } | ||
|
|
||
| private def selfDelegateMulticallBytesBadAritySpec : CompilationModel := { | ||
| name := "SelfDelegateMulticallBytesBadArity" | ||
| fields := [] | ||
| «constructor» := none | ||
| functions := [ | ||
| { name := "bad" | ||
| params := [ | ||
| { name := "calls", ty := ParamType.array ParamType.bytes } | ||
| ] | ||
| returnType := none | ||
| body := [ | ||
| Stmt.ecm (Compiler.Modules.Calls.selfDelegateMulticallBytesModule "calls") | ||
| [Expr.arrayLength "calls"], | ||
| Stmt.stop | ||
| ] | ||
| } | ||
| ] | ||
| } | ||
|
|
||
| private def selfDelegateMulticallBytesEmptyParamSpec : CompilationModel := { | ||
| name := "SelfDelegateMulticallBytesEmptyParam" | ||
| fields := [] | ||
| «constructor» := none | ||
| functions := [ | ||
| { name := "bad" | ||
| params := [ | ||
| { name := "calls", ty := ParamType.array ParamType.bytes } | ||
| ] | ||
| returnType := none | ||
| body := [ | ||
| Compiler.Modules.Calls.selfDelegateMulticallBytes "", | ||
| Stmt.stop | ||
| ] | ||
| } | ||
| ] | ||
| } | ||
|
|
||
| private def selfDelegateMulticallBytesViewRejectedSpec : CompilationModel := { | ||
| name := "SelfDelegateMulticallBytesViewRejected" | ||
| fields := [] | ||
| «constructor» := none | ||
| functions := [ | ||
| { name := "multicall" | ||
| params := [ | ||
| { name := "calls", ty := ParamType.array ParamType.bytes } | ||
| ] | ||
| returnType := none | ||
| isView := true | ||
| body := [ | ||
| Compiler.Modules.Calls.selfDelegateMulticallBytes "calls", | ||
| Stmt.stop | ||
| ] | ||
| } | ||
| ] | ||
| } | ||
|
|
||
| unsafe def runTests : IO Unit := do | ||
| let yul ← | ||
| expectCompileToYul "self-delegate multicall bytes smoke spec" selfDelegateMulticallBytesSmokeSpec | ||
| expectTrue "self-delegate multicall walks bytes[] calldata offsets" | ||
| (contains yul "for {" && | ||
| contains yul "let __mc_i := 0" && | ||
| contains yul "lt(__mc_i, calls_length)" && | ||
| contains yul "let __mc_rel_offset := calldataload(add(calls_data_offset, mul(__mc_i, 32)))" && | ||
| contains yul "if lt(__mc_rel_offset, mul(calls_length, 32)) {" && | ||
| contains yul "if gt(__mc_rel_offset, sub(not(0), calls_data_offset)) {" && | ||
| contains yul "let __mc_head_offset := add(calls_data_offset, __mc_rel_offset)" && | ||
| contains yul "let __mc_data_size := calldataload(__mc_head_offset)" && | ||
| contains yul "let __mc_data_offset := add(__mc_head_offset, 32)") | ||
| expectTrue "self-delegate multicall copies each bytes payload and delegatecalls address()" | ||
| (contains yul "calldatacopy(__mc_ptr, __mc_data_offset, __mc_data_size)" && | ||
| contains yul "delegatecall(gas(), address(), __mc_ptr, __mc_data_size, 0, 0)") | ||
| expectTrue "self-delegate multicall forwards revert returndata exactly" | ||
| (contains yul "let __mc_rds := returndatasize()" && | ||
| contains yul "returndatacopy(0, 0, __mc_rds)" && | ||
| contains yul "revert(0, __mc_rds)") | ||
| expectCompileErrorContains | ||
| "self-delegate multicall ECM rejects invalid argument counts" | ||
| selfDelegateMulticallBytesBadAritySpec | ||
| "uses ECM 'selfDelegateMulticallBytes' with 1 arguments but it expects 0" | ||
| expectCompileErrorContains | ||
| "self-delegate multicall rejects empty parameter names" | ||
| selfDelegateMulticallBytesEmptyParamSpec | ||
| "selfDelegateMulticallBytes: arrayParam must be non-empty" | ||
| expectCompileErrorContains | ||
| "self-delegate multicall remains rejected from view functions" | ||
| selfDelegateMulticallBytesViewRejectedSpec | ||
| "function 'multicall' is marked view but writes state" | ||
| let report := emitTrustReportJson [selfDelegateMulticallBytesSmokeSpec] | ||
| expectTrue "self-delegate multicall trust report surfaces scoped multicall assumption without proxy boundary" | ||
| (contains report "\"module\":\"selfDelegateMulticallBytes\"" && | ||
| contains report "\"assumption\":\"self_delegate_multicall_bytes_revert_bubbling\"" && | ||
| contains report "\"boundaryClass\":\"abiBoundary\"" && | ||
| contains report "\"notModeledProxyUpgradeability\":[]" && | ||
| ! (contains report "\"delegatecall\"")) | ||
|
|
||
| #eval! runTests | ||
|
|
||
| end Compiler.Modules.CallsTest |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Wrong bytes element head base
High Severity
Each
bytes[]offset in calldata is relative to the array length word, but__mc_head_offsetadds the loaded offset to{arrayParam}_data_offset, whichgenDynamicParamLoadsplaces 32 bytes after that length word. Standard Solidity-encoded multicall calldata can resolve the wrong element head, socalldatacopyanddelegatecallmay use incorrect payload bytes.Reviewed by Cursor Bugbot for commit c199d98. Configure here.
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.
Re: "Wrong bytes element head base" (High) — this is a false positive.
Per the Solidity ABI spec,
bytes[]encodes asenc(k) enc((X[0],…,X[k-1])). The element head offsets live inside the inner tupleenc((X[0],…)), and tuple offsets are measured from the start of that tuple encoding — i.e. from the first head word, which is the byte immediately after the length word — not from the length word itself.{arrayParam}_data_offsetis exactly that position (the first head word, 32 bytes past the length word, as the finding itself notes). So__mc_head_offset = data_offset + __mc_rel_offsetresolves the correct element head.Worked example (
bytes[]of two 2-byte elements, length word at byteP):P; heads atP+0x20,P+0x40;X[0]len atP+0x60,X[1]len atP+0xA0.data_offset = P+0x20. Encodedhead[0] = 0x40(relative to tuple start), sodata_offset + 0x40 = P+0x60 = X[0]✓.Pas base (as the finding suggests) would giveP+0x40 = head[1]— i.e. that base is the incorrect one.This is corroborated by the in-code guard
if __mc_rel_offset < arrayLength*32 then revert: the minimum valid relative offset equals the size of the heads table only when offsets are measured from the heads-block start (=data_offset). The emission additionally guards against pre-add wraparound (__mc_rel_offset > not(0) - data_offset), out-of-range head (> calldatasize-32), and out-of-range payload (> calldatasize - data_offset). No fix needed.