Ethereum: add optimized verification endpoint to the core bridge #3366
Conversation
gator-boi
force-pushed
the
ethereum/optimizations
branch
from
4a4ced1
to
8294537
Compare
| uint32 guardianSetIndex | ||
| ) internal view returns (bytes memory signedTransfer) { | ||
| // construct `TransferWithPayload` Wormhole message | ||
| Structs.VM memory vm; |
There was a problem hiding this comment.
If you're ok with it, would you mind renaming vm --> vm_? It just makes me cringe a bit to shadow the forge test vm cheatcode god object.
| vm.consistencyLevel = 15; | ||
| vm.payload = payload; | ||
|
|
||
| // encode the bservation |
There was a problem hiding this comment.
| // encode the bservation | |
| // encode the observation |
| @@ -161,4 +256,69 @@ contract TestMessages is Test { | |||
| assertEq(valid, false); | |||
| assertEq(reason, "vm.hash doesn't match body"); | |||
| } | |||
|
|
|||
| function testParseGuardianSetOptimized(uint8 guardianCount) public view { | |||
| vm.assume(guardianCount > 0 && guardianCount <= 19); | |||
There was a problem hiding this comment.
| vm.assume(guardianCount > 0 && guardianCount <= 19); | |
| guardianCount = bound(guardianCount, 0, 19); |
Per the forge docs for the assume cheatcode, for integers, it is best to use the bound cheatcode when fuzzing.
The way assume works with the fuzzer is that it tosses every fuzz test where the assume condition is false. Alternatively, bound simply constrains the integer within the given range. At high fuzz runs, more assumes will cause the fuzz test run to fail with max rejects.
gator-boi
force-pushed
the
ethereum/optimizations
branch
4 times, most recently
from
3122859
to
5bd07f9
Compare
| function parseGuardianSet(bytes calldata guardianSetData) public pure returns (Structs.GuardianSet memory guardianSet) { | ||
| // Fetch the guardian set length. | ||
| uint256 endGuardianKeyIndex = guardianSetData.length - 4; | ||
| uint256 guardianCount = endGuardianKeyIndex / 20; |
There was a problem hiding this comment.
I think it would be nice to specify that this 20 is the size of an address, perhaps with an ADDRESS_SIZE constant or something equivalent.
| /// @dev Obtain the current guardianSet for the guardianSetIndex provided | ||
| Structs.GuardianSet memory guardianSet = getGuardianSet(vm.guardianSetIndex); | ||
|
|
||
| function verifyVMInternal(Structs.VM memory vm, Structs.GuardianSet memory guardianSet, bool checkHash) internal view returns (bool valid, string memory reason) { |
There was a problem hiding this comment.
I wonder, wouldn't it be better to actually have observer functions that let you parse a guardian key just in time instead of reading them all into an unpacked struct array like this?
djb15
force-pushed
the
ethereum/optimizations
branch
from
71e5682
to
5b94fb0
Compare
| } | ||
| _state.guardianSets[index] = set; | ||
| _state.guardianSets[index] = set; |
The purpose of this PR is to add an optimized
parseAndVerifyendpoint toMessages.solalong with various other optimizations. See the following table for the estimated gas savings: