Skip to content

Commit

Permalink
evm: optimize parseVM
Browse files Browse the repository at this point in the history
  • Loading branch information
gator-boi committed Oct 4, 2023
1 parent 8294537 commit 5bd07f9
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 53 deletions.
2 changes: 1 addition & 1 deletion ethereum/contracts/Getters.sol
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ contract Getters is State {
uint256 guardianCount = guardianSet.keys.length;
for (uint256 i = 0; i < guardianCount;) {
encodedGuardianSet = abi.encodePacked(encodedGuardianSet, guardianSet.keys[i]);
unchecked { i += 1; }
unchecked { ++i; }
}
encodedGuardianSet = abi.encodePacked(encodedGuardianSet, guardianSet.expirationTime);
}
Expand Down
2 changes: 1 addition & 1 deletion ethereum/contracts/GovernanceStructs.sol
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ contract GovernanceStructs {
uint8 action;
uint16 chain;

Structs.GuardianSet newGuardianSet;
Structs.GuardianSet newGuardianSet;
uint32 newGuardianSetIndex;
}

Expand Down
90 changes: 41 additions & 49 deletions ethereum/contracts/Messages.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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;
}
}

/*
Expand All @@ -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");
}

/**
Expand Down
4 changes: 2 additions & 2 deletions ethereum/contracts/Setters.sol
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ contract Setters is State {
uint setLength = set.keys.length;
for (uint i = 0; i < setLength;) {
require(set.keys[i] != address(0), "Invalid key");
unchecked { i += 1; }
unchecked { ++i; }
}
_state.guardianSets[index] = set;
_state.guardianSets[index] = set;
}

function setInitialized(address implementatiom) internal {
Expand Down
9 changes: 9 additions & 0 deletions ethereum/forge-test/Messages.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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);
}
}
}
1 change: 1 addition & 0 deletions ethereum/forge-test/MessagesRV.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 5bd07f9

Please sign in to comment.