feat(#3): support call encodings#50
feat(#3): support call encodings#50re1ro wants to merge 1 commit intofeature/typed-encoder-step2from
Conversation
re1ro
commented
Oct 31, 2025
re1ro
commented
- enable CallWithSelector and CallWithSignature encoding types with validation
- extend calldata tests to cover selector and signature based calls
- enable CallWithSelector and CallWithSignature encoding types with validation - extend calldata tests to cover selector and signature based calls
There was a problem hiding this comment.
Pull Request Overview
This PR implements CallWithSelector and CallWithSignature encoding types for the TypedEncoder library, enabling dynamic construction of contract call data at runtime. These encoding types allow encoding function calls by either providing a bytes4 selector directly or computing it from a function signature string.
- Adds validation logic in the
encode()function to verify structural requirements for both call encoding types - Implements
_encodeCallWithSelector()and_encodeCallWithSignature()helper functions (implementation details not visible in diff) - Removes CallWithSelector/CallWithSignature from the unimplemented encoding types list
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/lib/TypedEncoder.sol | Adds validation and routing logic for CallWithSelector and CallWithSignature encoding types in the encode() function |
| test/libs/TypedEncoderCalldata.t.sol | Adds comprehensive test coverage for CallWithSelector and CallWithSignature encoding types with various parameter configurations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (chunk.primitives.length != 1 || chunk.structs.length != 1 || chunk.arrays.length != 0) { | ||
| revert InvalidCallEncodingStructure(); | ||
| } | ||
| Primitive memory selectorPrim = chunk.primitives[0]; |
There was a problem hiding this comment.
The validation checks selectorPrim.data.length != 4, but this checks the length of the bytes array itself, not the actual selector content. When using abi.encodePacked(bytes4), the resulting bytes array has a length field followed by 4 bytes of data. The check should validate that the data contains exactly 4 bytes of selector data. Consider using assembly to extract and validate the actual 4-byte content, or document that data should be created with abi.encodePacked(selector) which produces a 4-byte array.
| Primitive memory selectorPrim = chunk.primitives[0]; | |
| Primitive memory selectorPrim = chunk.primitives[0]; | |
| // selectorPrim.data must be constructed using abi.encodePacked(bytes4) to ensure length == 4 |
| if (s.encodingType == EncodingType.CallWithSelector) { | ||
| // Validate CallWithSelector encoding structure before forwarding | ||
| if (s.chunks.length != 1) { | ||
| revert InvalidCallEncodingStructure(); | ||
| } | ||
| Chunk memory chunk = s.chunks[0]; | ||
| if (chunk.primitives.length != 1 || chunk.structs.length != 1 || chunk.arrays.length != 0) { | ||
| revert InvalidCallEncodingStructure(); | ||
| } | ||
| Primitive memory selectorPrim = chunk.primitives[0]; | ||
| if (selectorPrim.isDynamic || selectorPrim.data.length != 4) { | ||
| revert InvalidCallEncodingStructure(); | ||
| } | ||
| return _encodeCallWithSelector(s); | ||
| } | ||
| if (s.encodingType == EncodingType.CallWithSignature) { | ||
| // Validate CallWithSignature encoding structure before forwarding | ||
| if (s.chunks.length != 1) { | ||
| revert InvalidCallEncodingStructure(); | ||
| } | ||
| Chunk memory chunk = s.chunks[0]; | ||
| if (chunk.primitives.length != 1 || chunk.structs.length != 1 || chunk.arrays.length != 0) { | ||
| revert InvalidCallEncodingStructure(); | ||
| } | ||
| if (!chunk.primitives[0].isDynamic) { | ||
| revert InvalidCallEncodingStructure(); | ||
| } | ||
| return _encodeCallWithSignature(s); | ||
| } |
There was a problem hiding this comment.
Why not do the validations inside the corresponding _encode method? These validations right now don't happen for _encodeChunkFields, why?
|
|
||
| // ============ Section 4: Error Cases ============ | ||
|
|
||
| function testCallWithSelectorInvalidStructure() public { |
There was a problem hiding this comment.
Why this test and all below skipped? Isn't the error there now?
2 participants
