From 062bf6fb5b675f61ed5d4fce36d79ff5463cba9f Mon Sep 17 00:00:00 2001 From: gator-boi Date: Wed, 4 Oct 2023 11:18:05 -0500 Subject: [PATCH] evm: optimize parseVM --- ethereum/contracts/Messages.sol | 90 +++++++++++++--------------- ethereum/forge-test/Messages.t.sol | 9 +++ ethereum/forge-test/MessagesRV.t.sol | 1 + 3 files changed, 51 insertions(+), 49 deletions(-) diff --git a/ethereum/contracts/Messages.sol b/ethereum/contracts/Messages.sol index 01d815659b..a043d982ff 100644 --- a/ethereum/contracts/Messages.sol +++ b/ethereum/contracts/Messages.sol @@ -6,11 +6,11 @@ pragma experimental ABIEncoderV2; import "./Getters.sol"; import "./Structs.sol"; -import "./libraries/external/BytesLib.sol"; +import "./libraries/relayer/BytesParsing.sol"; contract Messages is Getters { - using BytesLib for bytes; + using BytesParsing for bytes; function parseAndVerifyVMOptimized( bytes calldata encodedVM, @@ -23,7 +23,6 @@ contract Messages is Getters { "invalid guardian set" ); - // TODO: Optimize parsing function. vm = parseVM(encodedVM); // Verify that the VM is signed with the same guardian set that was specified. @@ -39,15 +38,17 @@ contract Messages is Getters { guardianSet = Structs.GuardianSet({ keys : new address[](guardianCount), - expirationTime : guardianSetData.toUint32(endGuardianKeyIndex) + expirationTime : 0 }); + (guardianSet.expirationTime, ) = guardianSetData.asUint32Unchecked(endGuardianKeyIndex); + uint256 offset = 0; for(uint256 i = 0; i < guardianCount;) { + (guardianSet.keys[i], offset) = guardianSetData.asAddressUnchecked(offset); unchecked { - guardianSet.keys[i] = guardianSetData.toAddress(i * 20); - i += 1; + ++i; } - } + } } /// @dev parseAndVerifyVM serves to parse an encodedVM and wholy validate it for consumption @@ -173,46 +174,46 @@ contract Messages is Getters { return (false, "VM signature invalid"); } - unchecked { i += 1; } + unchecked { ++i; } } /// If we are here, we've validated that the provided signatures are valid for the provided guardianSet return (true, ""); - } + } /** * @dev parseVM serves to parse an encodedVM into a vm struct * - it intentionally performs no validation functions, it simply parses raw into a struct */ - function parseVM(bytes memory encodedVM) public pure virtual returns (Structs.VM memory vm) { - uint index = 0; + function parseVM(bytes memory encodedVM) public view virtual returns (Structs.VM memory vm) { + uint256 offset = 0; - vm.version = encodedVM.toUint8(index); - index += 1; // SECURITY: Note that currently the VM.version is not part of the hash // and for reasons described below it cannot be made part of the hash. // This means that this field's integrity is not protected and cannot be trusted. // This is not a problem today since there is only one accepted version, but it // could be a problem if we wanted to allow other versions in the future. - require(vm.version == 1, "VM version incompatible"); + (vm.version, offset) = encodedVM.asUint8Unchecked(offset); + require(vm.version == 1, "invalid payload id"); - vm.guardianSetIndex = encodedVM.toUint32(index); - index += 4; + // Guardian set index. + (vm.guardianSetIndex, offset) = encodedVM.asUint32Unchecked(offset); + + // Parse sigs. + uint256 signersLen; + (signersLen, offset) = encodedVM.asUint8Unchecked(offset); - // Parse Signatures - uint256 signersLen = encodedVM.toUint8(index); - index += 1; vm.signatures = new Structs.Signature[](signersLen); - for (uint i = 0; i < signersLen; i++) { - vm.signatures[i].guardianIndex = encodedVM.toUint8(index); - index += 1; - - vm.signatures[i].r = encodedVM.toBytes32(index); - index += 32; - vm.signatures[i].s = encodedVM.toBytes32(index); - index += 32; - vm.signatures[i].v = encodedVM.toUint8(index) + 27; - index += 1; + for (uint i = 0; i < signersLen;) { + (vm.signatures[i].guardianIndex, offset) = encodedVM.asUint8Unchecked(offset); + (vm.signatures[i].r, offset) = encodedVM.asBytes32Unchecked(offset); + (vm.signatures[i].s, offset) = encodedVM.asBytes32Unchecked(offset); + (vm.signatures[i].v, offset) = encodedVM.asUint8Unchecked(offset); + + unchecked { + vm.signatures[i].v += 27; + ++i; + } } /* @@ -222,29 +223,20 @@ contract Messages is Getters { Changing it could result into two different hashes for the same observation. But xDapps rely on the hash of an observation for replay protection. */ - bytes memory body = encodedVM.slice(index, encodedVM.length - index); + bytes memory body; + (body, ) = encodedVM.sliceUnchecked(offset, encodedVM.length - offset); vm.hash = keccak256(abi.encodePacked(keccak256(body))); // Parse the body - vm.timestamp = encodedVM.toUint32(index); - index += 4; - - vm.nonce = encodedVM.toUint32(index); - index += 4; - - vm.emitterChainId = encodedVM.toUint16(index); - index += 2; - - vm.emitterAddress = encodedVM.toBytes32(index); - index += 32; - - vm.sequence = encodedVM.toUint64(index); - index += 8; - - vm.consistencyLevel = encodedVM.toUint8(index); - index += 1; - - vm.payload = encodedVM.slice(index, encodedVM.length - index); + (vm.timestamp, offset) = encodedVM.asUint32Unchecked(offset); + (vm.nonce, offset) = encodedVM.asUint32Unchecked(offset); + (vm.emitterChainId, offset) = encodedVM.asUint16Unchecked(offset); + (vm.emitterAddress, offset) = encodedVM.asBytes32Unchecked(offset); + (vm.sequence, offset) = encodedVM.asUint64Unchecked(offset); + (vm.consistencyLevel, offset) = encodedVM.asUint8Unchecked(offset); + (vm.payload, offset) = encodedVM.sliceUnchecked(offset, encodedVM.length - offset); + + require(encodedVM.length == offset, "invalid payload length"); } /** diff --git a/ethereum/forge-test/Messages.t.sol b/ethereum/forge-test/Messages.t.sol index e61a6bfaea..5f47824862 100644 --- a/ethereum/forge-test/Messages.t.sol +++ b/ethereum/forge-test/Messages.t.sol @@ -96,7 +96,9 @@ contract TestMessages is Test { } function setUp() public { + console.log("here?"); messages = new ExportedMessages(); + console.log("or?"); wormholeSimulator = new WormholeSigner(); setupSingleGuardian(); setupMultiGuardian(); @@ -320,5 +322,12 @@ contract TestMessages is Test { assertEq(vm_.payload, vmOptimized.payload); assertEq(vm_.guardianSetIndex, vmOptimized.guardianSetIndex); assertEq(vm_.signatures.length, vmOptimized.signatures.length); + + for (uint256 i = 0; i < vm_.signatures.length; ++i) { + assertEq(vm_.signatures[i].guardianIndex, vmOptimized.signatures[i].guardianIndex); + assertEq(vm_.signatures[i].r, vmOptimized.signatures[i].r); + assertEq(vm_.signatures[i].s, vmOptimized.signatures[i].s); + assertEq(vm_.signatures[i].v, vmOptimized.signatures[i].v); + } } } diff --git a/ethereum/forge-test/MessagesRV.t.sol b/ethereum/forge-test/MessagesRV.t.sol index f74acc4e99..b41ab3a6bd 100644 --- a/ethereum/forge-test/MessagesRV.t.sol +++ b/ethereum/forge-test/MessagesRV.t.sol @@ -7,6 +7,7 @@ import "../contracts/Messages.sol"; import "../contracts/Setters.sol"; import "../contracts/Structs.sol"; import "forge-test/rv-helpers/TestUtils.sol"; +import "../contracts/libraries/external/BytesLib.sol"; contract TestMessagesRV is TestUtils { using BytesLib for bytes;