Import Solana related libs and changes#215
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (9)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis PR adds Solana mainnet integration to the socket protocol. Key additions include a ForwarderSolana contract for Solana cross-chain calls, cryptographic utilities for Ed25519 signature verification and Solana PDA derivation, Borsh codec libraries for serialization, and updates to the fees management system to support Solana-native withdrawals. The onCompleteData storage variable is removed from AppGatewayBase. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CounterAppGateway
participant Watcher
participant WritePrecompile
participant Forwarder as Forwarder(EVM/Solana)
User->>CounterAppGateway: withdrawCredits(chainSlug, token, credits, ...)
alt Is Solana Chain
CounterAppGateway->>WritePrecompile: _createSolanaDigestParams()
WritePrecompile->>WritePrecompile: decode SolanaInstruction
WritePrecompile->>BorshEncoder: encodeFunctionArgs()
BorshEncoder-->>WritePrecompile: packed function args
WritePrecompile->>Watcher: queue payload (Solana variant)
else Is EVM Chain
CounterAppGateway->>WritePrecompile: _createEvmDigestParams()
WritePrecompile->>Watcher: queue payload (EVM variant)
end
Watcher->>Forwarder: resolve via precompile
alt Solana
Forwarder->>ForwarderSolana: callSolana(payload, target)
ForwarderSolana->>Watcher: addPayloadData(rawPayload)
else EVM
Forwarder->>EVMForwarder: call target
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
contracts/evmx/helpers/solana-utils/SolanaSignature.sol (1)
7-16: Simplify by removing unnecessary intermediate variable.The
validvariable is assigned but immediately returned without any additional logic. This can be simplified.Apply this diff:
function verifyMessage( bytes32 public_key, bytes32 signature_r, bytes32 signature_s, bytes memory message ) public pure returns (bool) { - bool valid = Ed25519.verify(public_key, signature_r, signature_s, message); - - return valid; + return Ed25519.verify(public_key, signature_r, signature_s, message); }contracts/utils/common/Structs.sol (1)
223-229: Consider struct field ordering for gas optimization.The struct has two bool fields that could benefit from packing. Consider ordering fields to group smaller types together for better storage efficiency in future iterations.
For example:
struct PayloadFees { uint256 nativeFees; address refundAddress; address plug; bool isRefundEligible; bool isRefunded; }This groups the two bools at the end, which may enable better packing with adjacent storage if this struct is used in arrays or mappings.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
contracts/evmx/base/AppGatewayBase.sol(0 hunks)contracts/evmx/fees/Credit.sol(8 hunks)contracts/evmx/fees/FeesManager.sol(3 hunks)contracts/evmx/helpers/ForwarderSolana.sol(1 hunks)contracts/evmx/helpers/solana-utils/Ed25519.sol(1 hunks)contracts/evmx/helpers/solana-utils/Ed25519_pow.sol(1 hunks)contracts/evmx/helpers/solana-utils/Sha512.sol(1 hunks)contracts/evmx/helpers/solana-utils/SolanaPda.sol(1 hunks)contracts/evmx/helpers/solana-utils/SolanaSignature.sol(1 hunks)contracts/evmx/helpers/solana-utils/program-pda/FeesPlugPdas.sol(1 hunks)contracts/evmx/watcher/Watcher.sol(5 hunks)contracts/evmx/watcher/borsh-serde/BorshDecoder.sol(1 hunks)contracts/evmx/watcher/borsh-serde/BorshEncoder.sol(1 hunks)contracts/evmx/watcher/borsh-serde/BorshUtils.sol(1 hunks)contracts/evmx/watcher/precompiles/ReadPrecompile.sol(1 hunks)contracts/evmx/watcher/precompiles/SchedulePrecompile.sol(2 hunks)contracts/evmx/watcher/precompiles/WritePrecompile.sol(5 hunks)contracts/protocol/switchboard/SwitchboardBase.sol(1 hunks)contracts/utils/common/Constants.sol(0 hunks)contracts/utils/common/Structs.sol(2 hunks)foundry.toml(1 hunks)test/SetupTest.t.sol(3 hunks)test/apps/Counter.t.sol(1 hunks)test/apps/counter/CounterAppGateway.sol(1 hunks)
💤 Files with no reviewable changes (2)
- contracts/evmx/base/AppGatewayBase.sol
- contracts/utils/common/Constants.sol
🔇 Additional comments (15)
contracts/protocol/switchboard/SwitchboardBase.sol (1)
62-63: Clarify the TODO and verify it doesn't break signature verification.The TODO comment is vague ("api encode packed" is ambiguous—presumably
abi.encodePacked?). Switching encoding schemes changes the hash output and breaks signature verification, since signatures were signed over the currentabi.encode()output. This is a breaking change that requires coordination with off-chain signers.Clarify what needs to change, why, and what the migration path is. This should be tracked in an issue with context rather than an inline comment. Also, this seems orthogonal to the Solana integration scope of this PR—should it be in a separate change?
contracts/evmx/watcher/precompiles/SchedulePrecompile.sol (2)
30-30: LGTM!The error declaration is clear and follows the existing error pattern in the codebase.
145-150: LGTM!The deadline enforcement is correctly implemented. The function now properly validates that execution occurs within the valid window: after the scheduled time but before the deadline expires. This aligns with the deadline checks added to ReadPrecompile.
contracts/evmx/watcher/precompiles/ReadPrecompile.sol (2)
71-74: LGTM!The event emission was added to the setFees function. This is proper practice for tracking fee configuration changes.
67-69: LGTMThe deadline validation is correctly implemented and consistent with SchedulePrecompile.sol. The
DeadlinePassed()error is properly defined in common/Errors.sol and will resolve correctly through the import on line 8.contracts/utils/common/Structs.sol (2)
202-220: LGTM!The Solana read types are well-designed for flexible account reading with both predefined and generic schema support. The use of keccak256 for schema name hashing is appropriate for efficient comparisons.
232-248: LGTM!The structs are well-designed for their purposes.
SponsoredPayloadFeesis simple and clear.MessageOverridescontains all necessary fields for message configuration with appropriate types.test/apps/Counter.t.sol (1)
87-88: LGTM!The assertion correctly reflects the updated callback behavior. With the new async callback flow using
then(), theonIncrementCompletecallback is properly executed, settingincrementedtotrue. This test now validates the correct behavior.test/apps/counter/CounterAppGateway.sol (1)
29-33: LGTM!The migration from
abi.encodeWithSelectortothen()correctly implements the new async callback architecture. Usingbytes("")as the callback data is appropriate sinceonIncrementCompletedoesn't require any parameters. This is cleaner than the previous storage-based completion approach.contracts/evmx/watcher/Watcher.sol (5)
134-137: LGTM!The cancellation guard correctly prevents processing of cancelled payloads with an early return. This is the appropriate behavior since attempting to resolve a cancelled payload should be a no-op rather than an error.
141-147: LGTM!The change correctly captures the success result from
_markResolvedand short-circuits if resolution fails. This prevents incorrect fee settlement and event emission when promise resolution fails, which is the correct behavior.
149-164: LGTM!Removing the deadline check from
_markResolvedis the correct refactoring. Deadline validation is now properly handled by the individual precompiles (ReadPrecompile and SchedulePrecompile) during theirresolvePayloadcalls. This improves separation of concerns.
119-127: LGTM!The multi-line formatting of
abi.decodecalls improves readability without any semantic changes. This is consistent with Solidity formatting best practices.Also applies to: 166-174
59-72: The guard is sufficient—this check prevents double-setting effectively.The
payloadData.asyncPromise != address(0)sentinel works because:
payloadDatahas no reset/clear mechanism anywhere in the codebase- The guard checks the current state before overwriting, blocking subsequent calls
- Once
asyncPromiseis set (non-zero), it persists, ensuring the guard always triggers on retryThe design uses struct field membership as the sentinel rather than an explicit state flag. While an explicit boolean flag would be marginally clearer, the current approach is functionally sound and prevents all double-setting scenarios.
foundry.toml (1)
8-8: Confirm this optimizer configuration is appropriate for alpha development and establish production settings before mainnet deployment.The alpha-stage protocol status and
ffi = truesetting indicate this is a development configuration. Lowoptimizer_runs = 1with minimal optimizer steps (optimizerSteps = "u") is acceptable for faster iteration during active development, but this configuration will significantly increase gas costs for end users if deployed to production without adjustment.Before mainnet deployment, establish a production profile in foundry.toml with
optimizer_runs >= 200and comprehensive optimizer steps. Document why development uses minimal optimization.
| // convert EVM uint256 18 decimals to Solana uint64 6 decimals | ||
| function convertToSolanaUint64(uint256 amount) pure returns (uint64) { | ||
| uint256 scaledAmount = amount / 10 ** 12; | ||
| require(scaledAmount <= type(uint64).max, "Amount exceeds uint64 max"); | ||
| return uint64(scaledAmount); |
There was a problem hiding this comment.
Prevent loss when converting to Solana decimals
convertToSolanaUint64 floors values that are not multiples of 1e12, so withdrawing an amount like 1 wei burns the full credit but forwards 0 to Solana. This should revert on unsupported decimal precision so users never lose residual value. Add a remainder check before dividing.
function convertToSolanaUint64(uint256 amount) pure returns (uint64) {
- uint256 scaledAmount = amount / 10 ** 12;
+ if (amount % 10 ** 12 != 0) revert InvalidAmount();
+ uint256 scaledAmount = amount / 10 ** 12;
require(scaledAmount <= type(uint64).max, "Amount exceeds uint64 max");
return uint64(scaledAmount);
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In contracts/evmx/fees/Credit.sol around lines 456 to 460, the
convertToSolanaUint64 function currently floors values by dividing by 10**12
which can silently drop residual wei; add a remainder check before scaling so
the function reverts on unsupported precision. Specifically, require(amount %
(10**12) == 0, "Unsupported decimal precision") (use uint256 literal for the
modulus), then perform the division and keep the existing uint64 overflow
require; this prevents accidental loss of value by reverting when the input has
non-zero lower 12 decimals.
| GenericSchema memory schema, | ||
| bytes memory encodedData | ||
| ) internal pure returns (bytes[] memory) { | ||
| bytes[] memory decodedParams = new bytes[](schema.valuesTypeNames.length); | ||
| Data memory data = from(encodedData); | ||
|
|
||
| for (uint256 i = 0; i < schema.valuesTypeNames.length; i++) { | ||
| string memory typeName = schema.valuesTypeNames[i]; | ||
|
|
||
| if (keccak256(bytes(typeName)) == keccak256(bytes("u8"))) { | ||
| uint8 value = data.decodeU8(); | ||
| decodedParams[i] = abi.encode(value); | ||
| } else if (keccak256(bytes(typeName)) == keccak256(bytes("u16"))) { | ||
| uint16 value = data.decodeU16(); | ||
| decodedParams[i] = abi.encode(value); | ||
| } else if (keccak256(bytes(typeName)) == keccak256(bytes("u32"))) { | ||
| uint32 value = data.decodeU32(); | ||
| decodedParams[i] = abi.encode(value); | ||
| } else if (keccak256(bytes(typeName)) == keccak256(bytes("u64"))) { | ||
| uint64 value = data.decodeU64(); | ||
| decodedParams[i] = abi.encode(value); | ||
| } else if (keccak256(bytes(typeName)) == keccak256(bytes("u128"))) { | ||
| uint128 value = data.decodeU128(); | ||
| decodedParams[i] = abi.encode(value); | ||
| } else if (keccak256(bytes(typeName)) == keccak256(bytes("String"))) { | ||
| string memory value = data.decodeString(); | ||
| decodedParams[i] = abi.encode(value); | ||
| } | ||
| // Handle Vector types with variable length | ||
| else if (keccak256(bytes(typeName)) == keccak256(bytes("Vec<u8>"))) { | ||
| uint32 length; | ||
| uint8[] memory value; | ||
| (length, value) = decodeUint8Vec(data); | ||
| decodedParams[i] = abi.encode(value); | ||
| } else if (keccak256(bytes(typeName)) == keccak256(bytes("Vec<u16>"))) { | ||
| uint32 length; | ||
| uint16[] memory value; | ||
| (length, value) = decodeUint16Vec(data); | ||
| decodedParams[i] = abi.encode(value); | ||
| } else if (keccak256(bytes(typeName)) == keccak256(bytes("Vec<u32>"))) { | ||
| uint32 length; | ||
| uint32[] memory value; | ||
| (length, value) = decodeUint32Vec(data); | ||
| decodedParams[i] = abi.encode(value); | ||
| } else if (keccak256(bytes(typeName)) == keccak256(bytes("Vec<u64>"))) { | ||
| uint32 length; | ||
| uint64[] memory value; | ||
| (length, value) = decodeUint64Vec(data); | ||
| decodedParams[i] = abi.encode(value); | ||
| } else if (keccak256(bytes(typeName)) == keccak256(bytes("Vec<u128>"))) { | ||
| uint32 length; | ||
| uint128[] memory value; | ||
| (length, value) = decodeUint128Vec(data); | ||
| decodedParams[i] = abi.encode(value); | ||
| } else if (keccak256(bytes(typeName)) == keccak256(bytes("Vec<String>"))) { | ||
| uint32 length; | ||
| string[] memory value; | ||
| (length, value) = decodeStringVec(data); | ||
| decodedParams[i] = abi.encode(value); | ||
| } | ||
| // Handle Array types with fixed length | ||
| else if (BorshUtils.startsWith(typeName, "[u8;")) { | ||
| uint256 length = BorshUtils.extractArrayLength(typeName); | ||
| uint8[] memory value = decodeUint8Array(data, length); | ||
| decodedParams[i] = abi.encode(value); | ||
| } else if (BorshUtils.startsWith(typeName, "[u16;")) { | ||
| uint256 length = BorshUtils.extractArrayLength(typeName); | ||
| uint16[] memory value = decodeUint16Array(data, length); | ||
| decodedParams[i] = abi.encode(value); | ||
| } else if (BorshUtils.startsWith(typeName, "[u32;")) { | ||
| uint256 length = BorshUtils.extractArrayLength(typeName); | ||
| uint32[] memory value = decodeUint32Array(data, length); | ||
| decodedParams[i] = abi.encode(value); | ||
| } else if (BorshUtils.startsWith(typeName, "[u64;")) { | ||
| uint256 length = BorshUtils.extractArrayLength(typeName); | ||
| uint64[] memory value = decodeUint64Array(data, length); | ||
| decodedParams[i] = abi.encode(value); | ||
| } else if (BorshUtils.startsWith(typeName, "[u128;")) { | ||
| uint256 length = BorshUtils.extractArrayLength(typeName); | ||
| uint128[] memory value = decodeUint128Array(data, length); | ||
| decodedParams[i] = abi.encode(value); | ||
| } else if (BorshUtils.startsWith(typeName, "[String;")) { | ||
| uint256 length = BorshUtils.extractArrayLength(typeName); | ||
| string[] memory value = decodeStringArray(data, length); | ||
| decodedParams[i] = abi.encode(value); | ||
| } else { | ||
| revert("Unsupported type"); | ||
| } | ||
| } | ||
|
|
||
| return decodedParams; | ||
| } |
There was a problem hiding this comment.
Reject trailing bytes before returning.
This should call data.done() before returning so that extra bytes trigger a parse failure instead of being silently ignored. Right now a caller can append junk data and still pass validation, which breaks schema integrity.
for (uint256 i = 0; i < schema.valuesTypeNames.length; i++) {
string memory typeName = schema.valuesTypeNames[i];
@@
- return decodedParams;
+ data.done();
+ return decodedParams;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| GenericSchema memory schema, | |
| bytes memory encodedData | |
| ) internal pure returns (bytes[] memory) { | |
| bytes[] memory decodedParams = new bytes[](schema.valuesTypeNames.length); | |
| Data memory data = from(encodedData); | |
| for (uint256 i = 0; i < schema.valuesTypeNames.length; i++) { | |
| string memory typeName = schema.valuesTypeNames[i]; | |
| if (keccak256(bytes(typeName)) == keccak256(bytes("u8"))) { | |
| uint8 value = data.decodeU8(); | |
| decodedParams[i] = abi.encode(value); | |
| } else if (keccak256(bytes(typeName)) == keccak256(bytes("u16"))) { | |
| uint16 value = data.decodeU16(); | |
| decodedParams[i] = abi.encode(value); | |
| } else if (keccak256(bytes(typeName)) == keccak256(bytes("u32"))) { | |
| uint32 value = data.decodeU32(); | |
| decodedParams[i] = abi.encode(value); | |
| } else if (keccak256(bytes(typeName)) == keccak256(bytes("u64"))) { | |
| uint64 value = data.decodeU64(); | |
| decodedParams[i] = abi.encode(value); | |
| } else if (keccak256(bytes(typeName)) == keccak256(bytes("u128"))) { | |
| uint128 value = data.decodeU128(); | |
| decodedParams[i] = abi.encode(value); | |
| } else if (keccak256(bytes(typeName)) == keccak256(bytes("String"))) { | |
| string memory value = data.decodeString(); | |
| decodedParams[i] = abi.encode(value); | |
| } | |
| // Handle Vector types with variable length | |
| else if (keccak256(bytes(typeName)) == keccak256(bytes("Vec<u8>"))) { | |
| uint32 length; | |
| uint8[] memory value; | |
| (length, value) = decodeUint8Vec(data); | |
| decodedParams[i] = abi.encode(value); | |
| } else if (keccak256(bytes(typeName)) == keccak256(bytes("Vec<u16>"))) { | |
| uint32 length; | |
| uint16[] memory value; | |
| (length, value) = decodeUint16Vec(data); | |
| decodedParams[i] = abi.encode(value); | |
| } else if (keccak256(bytes(typeName)) == keccak256(bytes("Vec<u32>"))) { | |
| uint32 length; | |
| uint32[] memory value; | |
| (length, value) = decodeUint32Vec(data); | |
| decodedParams[i] = abi.encode(value); | |
| } else if (keccak256(bytes(typeName)) == keccak256(bytes("Vec<u64>"))) { | |
| uint32 length; | |
| uint64[] memory value; | |
| (length, value) = decodeUint64Vec(data); | |
| decodedParams[i] = abi.encode(value); | |
| } else if (keccak256(bytes(typeName)) == keccak256(bytes("Vec<u128>"))) { | |
| uint32 length; | |
| uint128[] memory value; | |
| (length, value) = decodeUint128Vec(data); | |
| decodedParams[i] = abi.encode(value); | |
| } else if (keccak256(bytes(typeName)) == keccak256(bytes("Vec<String>"))) { | |
| uint32 length; | |
| string[] memory value; | |
| (length, value) = decodeStringVec(data); | |
| decodedParams[i] = abi.encode(value); | |
| } | |
| // Handle Array types with fixed length | |
| else if (BorshUtils.startsWith(typeName, "[u8;")) { | |
| uint256 length = BorshUtils.extractArrayLength(typeName); | |
| uint8[] memory value = decodeUint8Array(data, length); | |
| decodedParams[i] = abi.encode(value); | |
| } else if (BorshUtils.startsWith(typeName, "[u16;")) { | |
| uint256 length = BorshUtils.extractArrayLength(typeName); | |
| uint16[] memory value = decodeUint16Array(data, length); | |
| decodedParams[i] = abi.encode(value); | |
| } else if (BorshUtils.startsWith(typeName, "[u32;")) { | |
| uint256 length = BorshUtils.extractArrayLength(typeName); | |
| uint32[] memory value = decodeUint32Array(data, length); | |
| decodedParams[i] = abi.encode(value); | |
| } else if (BorshUtils.startsWith(typeName, "[u64;")) { | |
| uint256 length = BorshUtils.extractArrayLength(typeName); | |
| uint64[] memory value = decodeUint64Array(data, length); | |
| decodedParams[i] = abi.encode(value); | |
| } else if (BorshUtils.startsWith(typeName, "[u128;")) { | |
| uint256 length = BorshUtils.extractArrayLength(typeName); | |
| uint128[] memory value = decodeUint128Array(data, length); | |
| decodedParams[i] = abi.encode(value); | |
| } else if (BorshUtils.startsWith(typeName, "[String;")) { | |
| uint256 length = BorshUtils.extractArrayLength(typeName); | |
| string[] memory value = decodeStringArray(data, length); | |
| decodedParams[i] = abi.encode(value); | |
| } else { | |
| revert("Unsupported type"); | |
| } | |
| } | |
| return decodedParams; | |
| } | |
| GenericSchema memory schema, | |
| bytes memory encodedData | |
| ) internal pure returns (bytes[] memory) { | |
| bytes[] memory decodedParams = new bytes[](schema.valuesTypeNames.length); | |
| Data memory data = from(encodedData); | |
| for (uint256 i = 0; i < schema.valuesTypeNames.length; i++) { | |
| string memory typeName = schema.valuesTypeNames[i]; | |
| if (keccak256(bytes(typeName)) == keccak256(bytes("u8"))) { | |
| uint8 value = data.decodeU8(); | |
| decodedParams[i] = abi.encode(value); | |
| } else if (keccak256(bytes(typeName)) == keccak256(bytes("u16"))) { | |
| uint16 value = data.decodeU16(); | |
| decodedParams[i] = abi.encode(value); | |
| } else if (keccak256(bytes(typeName)) == keccak256(bytes("u32"))) { | |
| uint32 value = data.decodeU32(); | |
| decodedParams[i] = abi.encode(value); | |
| } else if (keccak256(bytes(typeName)) == keccak256(bytes("u64"))) { | |
| uint64 value = data.decodeU64(); | |
| decodedParams[i] = abi.encode(value); | |
| } else if (keccak256(bytes(typeName)) == keccak256(bytes("u128"))) { | |
| uint128 value = data.decodeU128(); | |
| decodedParams[i] = abi.encode(value); | |
| } else if (keccak256(bytes(typeName)) == keccak256(bytes("String"))) { | |
| string memory value = data.decodeString(); | |
| decodedParams[i] = abi.encode(value); | |
| } | |
| // Handle Vector types with variable length | |
| else if (keccak256(bytes(typeName)) == keccak256(bytes("Vec<u8>"))) { | |
| uint32 length; | |
| uint8[] memory value; | |
| (length, value) = decodeUint8Vec(data); | |
| decodedParams[i] = abi.encode(value); | |
| } else if (keccak256(bytes(typeName)) == keccak256(bytes("Vec<u16>"))) { | |
| uint32 length; | |
| uint16[] memory value; | |
| (length, value) = decodeUint16Vec(data); | |
| decodedParams[i] = abi.encode(value); | |
| } else if (keccak256(bytes(typeName)) == keccak256(bytes("Vec<u32>"))) { | |
| uint32 length; | |
| uint32[] memory value; | |
| (length, value) = decodeUint32Vec(data); | |
| decodedParams[i] = abi.encode(value); | |
| } else if (keccak256(bytes(typeName)) == keccak256(bytes("Vec<u64>"))) { | |
| uint32 length; | |
| uint64[] memory value; | |
| (length, value) = decodeUint64Vec(data); | |
| decodedParams[i] = abi.encode(value); | |
| } else if (keccak256(bytes(typeName)) == keccak256(bytes("Vec<u128>"))) { | |
| uint32 length; | |
| uint128[] memory value; | |
| (length, value) = decodeUint128Vec(data); | |
| decodedParams[i] = abi.encode(value); | |
| } else if (keccak256(bytes(typeName)) == keccak256(bytes("Vec<String>"))) { | |
| uint32 length; | |
| string[] memory value; | |
| (length, value) = decodeStringVec(data); | |
| decodedParams[i] = abi.encode(value); | |
| } | |
| // Handle Array types with fixed length | |
| else if (BorshUtils.startsWith(typeName, "[u8;")) { | |
| uint256 length = BorshUtils.extractArrayLength(typeName); | |
| uint8[] memory value = decodeUint8Array(data, length); | |
| decodedParams[i] = abi.encode(value); | |
| } else if (BorshUtils.startsWith(typeName, "[u16;")) { | |
| uint256 length = BorshUtils.extractArrayLength(typeName); | |
| uint16[] memory value = decodeUint16Array(data, length); | |
| decodedParams[i] = abi.encode(value); | |
| } else if (BorshUtils.startsWith(typeName, "[u32;")) { | |
| uint256 length = BorshUtils.extractArrayLength(typeName); | |
| uint32[] memory value = decodeUint32Array(data, length); | |
| decodedParams[i] = abi.encode(value); | |
| } else if (BorshUtils.startsWith(typeName, "[u64;")) { | |
| uint256 length = BorshUtils.extractArrayLength(typeName); | |
| uint64[] memory value = decodeUint64Array(data, length); | |
| decodedParams[i] = abi.encode(value); | |
| } else if (BorshUtils.startsWith(typeName, "[u128;")) { | |
| uint256 length = BorshUtils.extractArrayLength(typeName); | |
| uint128[] memory value = decodeUint128Array(data, length); | |
| decodedParams[i] = abi.encode(value); | |
| } else if (BorshUtils.startsWith(typeName, "[String;")) { | |
| uint256 length = BorshUtils.extractArrayLength(typeName); | |
| string[] memory value = decodeStringArray(data, length); | |
| decodedParams[i] = abi.encode(value); | |
| } else { | |
| revert("Unsupported type"); | |
| } | |
| } | |
| data.done(); | |
| return decodedParams; | |
| } |
🤖 Prompt for AI Agents
In contracts/evmx/watcher/borsh-serde/BorshDecoder.sol around lines 15 to 106,
the decoder returns decodedParams without verifying there are no leftover bytes;
call data.done() just before returning to enforce that all input was consumed
and cause a revert if trailing bytes remain, ensuring parse failures on
extra/junk data.
| } else if (BorshUtils.startsWith(typeName, "[u16;")) { | ||
| uint16[] memory abiDecodedArg = abi.decode(data, (uint16[])); | ||
| bytes memory borshEncodedArg = encodeUint16Array(abiDecodedArg); | ||
| functionArgsPacked = abi.encodePacked(functionArgsPacked, borshEncodedArg); | ||
| } else if (BorshUtils.startsWith(typeName, "[u32;")) { | ||
| uint32[] memory abiDecodedArg = abi.decode(data, (uint32[])); | ||
| bytes memory borshEncodedArg = encodeUint32Array(abiDecodedArg); | ||
| functionArgsPacked = abi.encodePacked(functionArgsPacked, borshEncodedArg); | ||
| } else if (BorshUtils.startsWith(typeName, "[u64;")) { | ||
| uint64[] memory abiDecodedArg = abi.decode(data, (uint64[])); | ||
| bytes memory borshEncodedArg = encodeUint64Array(abiDecodedArg); | ||
| functionArgsPacked = abi.encodePacked(functionArgsPacked, borshEncodedArg); | ||
| } else if (BorshUtils.startsWith(typeName, "[u128;")) { | ||
| uint128[] memory abiDecodedArg = abi.decode(data, (uint128[])); | ||
| bytes memory borshEncodedArg = encodeUint128Array(abiDecodedArg); | ||
| functionArgsPacked = abi.encodePacked(functionArgsPacked, borshEncodedArg); | ||
| } else if (BorshUtils.startsWith(typeName, "[String;")) { | ||
| string[] memory abiDecodedArg = abi.decode(data, (string[])); | ||
| bytes memory borshEncodedArg = encodeStringArray(abiDecodedArg); | ||
| functionArgsPacked = abi.encodePacked(functionArgsPacked, borshEncodedArg); | ||
| } else { |
There was a problem hiding this comment.
Enforce fixed-length Borsh array lengths
For every fixed-length array type except [u8;N] we never confirm that the decoded array length actually matches N. That means a caller can pass an undersized or oversized payload, we’ll happily pack whatever is provided, and the downstream Solana program will deserialize garbage (or fail) because Borsh expects a fixed-length sequence. This should revert immediately when the ABI input length does not match the declared schema. Add the same length check you already have for [u8;N] to the [u16;N]/[u32;N]/[u64;N]/[u128;N]/[String;N] branches so malformed data can’t slip through.
- } else if (BorshUtils.startsWith(typeName, "[u16;")) {
- uint16[] memory abiDecodedArg = abi.decode(data, (uint16[]));
+ } else if (BorshUtils.startsWith(typeName, "[u16;")) {
+ uint256 expectedLength = BorshUtils.extractArrayLength(typeName);
+ uint16[] memory abiDecodedArg = abi.decode(data, (uint16[]));
+ require(abiDecodedArg.length == expectedLength, "[u16;N] length mismatch");
bytes memory borshEncodedArg = encodeUint16Array(abiDecodedArg);
functionArgsPacked = abi.encodePacked(functionArgsPacked, borshEncodedArg);
} else if (BorshUtils.startsWith(typeName, "[u32;")) {
- uint32[] memory abiDecodedArg = abi.decode(data, (uint32[]));
+ uint256 expectedLength = BorshUtils.extractArrayLength(typeName);
+ uint32[] memory abiDecodedArg = abi.decode(data, (uint32[]));
+ require(abiDecodedArg.length == expectedLength, "[u32;N] length mismatch");
bytes memory borshEncodedArg = encodeUint32Array(abiDecodedArg);
functionArgsPacked = abi.encodePacked(functionArgsPacked, borshEncodedArg);
} else if (BorshUtils.startsWith(typeName, "[u64;")) {
- uint64[] memory abiDecodedArg = abi.decode(data, (uint64[]));
+ uint256 expectedLength = BorshUtils.extractArrayLength(typeName);
+ uint64[] memory abiDecodedArg = abi.decode(data, (uint64[]));
+ require(abiDecodedArg.length == expectedLength, "[u64;N] length mismatch");
bytes memory borshEncodedArg = encodeUint64Array(abiDecodedArg);
functionArgsPacked = abi.encodePacked(functionArgsPacked, borshEncodedArg);
} else if (BorshUtils.startsWith(typeName, "[u128;")) {
- uint128[] memory abiDecodedArg = abi.decode(data, (uint128[]));
+ uint256 expectedLength = BorshUtils.extractArrayLength(typeName);
+ uint128[] memory abiDecodedArg = abi.decode(data, (uint128[]));
+ require(abiDecodedArg.length == expectedLength, "[u128;N] length mismatch");
bytes memory borshEncodedArg = encodeUint128Array(abiDecodedArg);
functionArgsPacked = abi.encodePacked(functionArgsPacked, borshEncodedArg);
} else if (BorshUtils.startsWith(typeName, "[String;")) {
- string[] memory abiDecodedArg = abi.decode(data, (string[]));
+ uint256 expectedLength = BorshUtils.extractArrayLength(typeName);
+ string[] memory abiDecodedArg = abi.decode(data, (string[]));
+ require(abiDecodedArg.length == expectedLength, "[String;N] length mismatch");
bytes memory borshEncodedArg = encodeStringArray(abiDecodedArg);
functionArgsPacked = abi.encodePacked(functionArgsPacked, borshEncodedArg);
}🤖 Prompt for AI Agents
contracts/evmx/watcher/borsh-serde/BorshEncoder.sol around lines 127 to 147: the
fixed-length array branches for [u16;N], [u32;N], [u64;N], [u128;N], and
[String;N] do not validate that abi.decode produced exactly N elements; add the
same length validation you have for [u8;N]. Parse the expected N from typeName
(the digits between the semicolon and closing bracket), then
require(abiDecodedArg.length == N, "BorshEncoder: invalid fixed-length array
length") (cast lengths to uint256 as needed) before calling the encode*Array
function and packing, so malformed undersized/oversized inputs revert
immediately.
| // TODO: this is temporary, must be injected from function arguments | ||
| // bytes32 of Solana Socket address : 9vFEQ5e3xf4eo17WttfqmXmnqN3gUicrhFGppmmNwyqV | ||
| bytes32 hardcodedSocket = 0x84815e8ca2f6dad7e12902c39a51bc72e13c48139b4fb10025d94e7abea2969c; | ||
| // bytes32 of Solana transmitter address : pFCBP4bhqdSsrWUVTgqhPsLrfEdChBK17vgFM7TxjxQ | ||
| bytes32 transmitterSolana = 0x0c1a5886fe1093df9fc438c296f9f7275b7718b6bc0e156d8d336c58f083996d; | ||
| return | ||
| DigestParams( | ||
| // watcherPrecompileConfig__.sockets(params_.payloadHeader.getChainSlug()), // TODO: this does not work, for some reason it returns 0x000.... address | ||
| hardcodedSocket, | ||
| // toBytes32Format(transmitter_), | ||
| transmitterSolana, | ||
| payloadId_, | ||
| deadline_, | ||
| rawPayload_.overrideParams.callType, | ||
| gasLimit_, | ||
| rawPayload_.overrideParams.value, | ||
| payloadPacked, | ||
| rawPayload_.transaction.target, | ||
| abi.encode(toBytes32Format(appGateway_)), | ||
| bytes32(0), | ||
| bytes("") | ||
| ); |
There was a problem hiding this comment.
Stop hardcoding Solana socket/transmitter in digests
_createSolanaDigestParams bakes in fixed bytes32 values for the Solana socket and transmitter. Any environment whose watcher config differs from these literals (read: every non-dev deployment) will compute a different digest off-chain, so proofs can never verify and executions brick. This should read the Solana socket/transmitter from watcher configuration or from the payload parameters, the same way the EVM path already does. Until the sender-provided data populates these fields (or watcher__ exposes dedicated Solana getters), we can’t release this path.
2 participants
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Refactor