Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ethereum: Add verifyCurrentQuorum method to core bridge #3540

Draft
wants to merge 6 commits into
base: ethereum/optimizations
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions ethereum/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,17 @@ node_modules: package-lock.json
# When adding a new dependency, make sure to specify the exact commit hash, and
# the --no-git and --no-commit flags (see lib/forge-std below)
.PHONY: forge_dependencies
forge_dependencies: lib/forge-std lib/openzeppelin-contracts
forge_dependencies: lib/forge-std lib/openzeppelin-contracts lib/wormhole-solidity-sdk

lib/forge-std:
forge install foundry-rs/[email protected] --no-git --no-commit

lib/openzeppelin-contracts:
forge install openzeppelin/openzeppelin-contracts@0457042d93d9dfd760dbaa06a4d2f1216fdbe297 --no-git --no-commit

lib/wormhole-solidity-sdk:
forge install djb15/wormhole-solidity-sdk@78f4b5e1d1ce4c36e43522c9205eb1e92fde7106 --no-git --no-commit

dependencies: node_modules forge_dependencies

build: forge_dependencies node_modules ${SOURCE_FILES}
Expand Down Expand Up @@ -87,4 +90,4 @@ test-push0: dependencies
@if grep -qr --include \*.json PUSH0 ./build-forge; then echo "Contract uses PUSH0 instruction" 1>&2; exit 1; fi

clean:
rm -rf ganache.log .env node_modules build flattened build-forge ethers-contracts lib/forge-std lib/openzeppelin-contracts
rm -rf ganache.log .env node_modules build flattened build-forge ethers-contracts lib/forge-std lib/openzeppelin-contracts lib/wormhole-solidity-sdk
37 changes: 35 additions & 2 deletions ethereum/contracts/Messages.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ pragma experimental ABIEncoderV2;

import "./Getters.sol";
import "./Structs.sol";
import "./libraries/relayer/BytesParsing.sol";
import "wormhole-sdk/libraries/BytesParsing.sol";


contract Messages is Getters {
Expand Down Expand Up @@ -147,7 +147,7 @@ contract Messages is Getters {


/**
* @dev verifySignatures serves to validate arbitrary sigatures against an arbitrary guardianSet
* @dev verifySignatures serves to validate arbitrary signatures against an arbitrary guardianSet
* - it intentionally does not solve for expectations within guardianSet (you should use verifyVM if you need these protections)
* - it intentioanlly does not solve for quorum (you should use verifyVM if you need these protections)
* - it intentionally returns true when signatures is an empty set (you should use verifyVM if you need these protections)
Expand Down Expand Up @@ -187,6 +187,39 @@ contract Messages is Getters {
return (true, "");
}

/**
* @dev verifyCurrentQuorum serves to validate arbitrary signatures and check quorum against the current guardianSet
*/
function verifyCurrentQuorum(bytes32 hash, Structs.Signature[] memory signatures) public view returns (bool, string memory) {
uint32 gsi = getCurrentGuardianSetIndex();
Structs.GuardianSet memory guardianSet = getGuardianSet(gsi);
uint256 guardianCount = guardianSet.keys.length;

/**
* @dev Checks whether the guardianSet has zero keys
* WARNING: This keys check is critical to ensure the guardianSet has keys present AND to ensure
* that guardianSet key size doesn't fall to zero and negatively impact quorum assessment. If guardianSet
* key length is 0 and vm.signatures length is 0, this could compromise the integrity of both vm and
* signature verification.
*/
if(guardianCount == 0){
return (false, "invalid guardian set");
}

/**
* @dev We're using a fixed point number transformation with 1 decimal to deal with rounding.
* WARNING: This quorum check is critical to assessing whether we have enough Guardian signatures to validate a VM
* if making any changes to this, obtain additional peer review. If guardianSet key length is 0 and
* vm.signatures length is 0, this could compromise the integrity of both vm and signature verification.
*/
if (signatures.length < quorum(guardianCount)){
return (false, "no quorum");
}

/// @dev Verify the proposed vm.signatures against the guardianSet
return verifySignatures(hash, signatures, guardianSet);
}

/**
* @dev parseVM serves to parse an encodedVM into a vm struct
* - it intentionally performs no validation functions, it simply parses raw into a struct
Expand Down
2 changes: 2 additions & 0 deletions ethereum/contracts/interfaces/IWormhole.sol
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@ interface IWormhole {

function verifySignatures(bytes32 hash, Signature[] memory signatures, GuardianSet memory guardianSet) external pure returns (bool valid, string memory reason);

function verifyCurrentQuorum(bytes32 hash, Signature[] memory signatures) external view returns (bool valid, string memory response);

function parseVM(bytes memory encodedVM) external pure returns (VM memory vm);

function quorum(uint numGuardians) external pure returns (uint numSignaturesRequiredForQuorum);
Expand Down
132 changes: 132 additions & 0 deletions ethereum/forge-test/Messages.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -330,4 +330,136 @@ contract TestMessages is Test {
assertEq(vm_.signatures[i].v, vmOptimized.signatures[i].v);
}
}
/// @dev This test checks that quorum is reached in the happy path
function testFuzz_verifyCurrentQuorum(bytes memory encoded) public {
vm.assume(encoded.length > 0);

// Set the initial guardian set
address[] memory initialGuardians = new address[](1);
initialGuardians[0] = testGuardianPub;

// Create a guardian set
Structs.GuardianSet memory initialGuardianSet = Structs.GuardianSet({
keys: initialGuardians,
expirationTime: 0
});

messages.storeGuardianSetPub(initialGuardianSet, uint32(0));

bytes32 message = keccak256(encoded);

// Generate legitimate signature.
Structs.Signature memory goodSignature;
(goodSignature.v, goodSignature.r, goodSignature.s) = vm.sign(testGuardian, message);
assertEq(ecrecover(message, goodSignature.v, goodSignature.r, goodSignature.s), vm.addr(testGuardian));
goodSignature.guardianIndex = 0;

// Attempt to verify signatures.
Structs.Signature[] memory sigs = new Structs.Signature[](1);
sigs[0] = goodSignature;

(bool valid, string memory reason) = messages.verifyCurrentQuorum(message, sigs);
assertEq(valid, true);
assertEq(bytes(reason).length, 0);
}

/// @dev This test checks that quorum will not be reached if there are no guardians in the current guardian set
function testFuzz_verifyCurrentQuorum_noGuardians(bytes memory encoded) public {
vm.assume(encoded.length > 0);

bytes32 message = keccak256(encoded);

// Generate legitimate signature.
Structs.Signature memory goodSignature;
(goodSignature.v, goodSignature.r, goodSignature.s) = vm.sign(testGuardian, message);
assertEq(ecrecover(message, goodSignature.v, goodSignature.r, goodSignature.s), vm.addr(testGuardian));
goodSignature.guardianIndex = 0;

// Attempt to verify signatures.
Structs.Signature[] memory sigs = new Structs.Signature[](1);
sigs[0] = goodSignature;

(bool valid, string memory reason) = messages.verifyCurrentQuorum(message, sigs);
assertEq(valid, false);
assertEq(abi.encodePacked(reason), abi.encodePacked("invalid guardian set"));
}

/// @dev This test checks that quorum will not be reached if there is >0 invalid signatures in any position
function testFuzz_verifyCurrentQuorum_invalidSignature(bytes memory encoded, uint8 numSigs, uint256 invalidSigIndex) public {
vm.assume(encoded.length > 0);
vm.assume(numSigs != 0);
invalidSigIndex = bound(invalidSigIndex, 0, numSigs - 1);

// Set the initial guardian set
address[] memory initialGuardians = new address[](numSigs);

bytes32 message = keccak256(encoded);

Structs.Signature[] memory signatures = new Structs.Signature[](numSigs);

for (uint8 i = 0; i < numSigs; ++i) {
(address addr, uint256 key) = makeAddrAndKey(string(abi.encode(i)));
initialGuardians[i] = addr;

// Create a single invalid signature
if (i == invalidSigIndex) {
(addr, key) = makeAddrAndKey(string(abi.encode(type(uint64).max)));
(signatures[i].v, signatures[i].r, signatures[i].s) = vm.sign(key, message);
signatures[i].guardianIndex = i;
continue;
}

(signatures[i].v, signatures[i].r, signatures[i].s) = vm.sign(key, message);
signatures[i].guardianIndex = i;
}

// Create a guardian set
Structs.GuardianSet memory initialGuardianSet = Structs.GuardianSet({
keys: initialGuardians,
expirationTime: 0
});

messages.storeGuardianSetPub(initialGuardianSet, uint32(0));

(bool valid, string memory reason) = messages.verifyCurrentQuorum(message, signatures);
assertEq(valid, false);
assertEq(abi.encodePacked(reason), abi.encodePacked("VM signature invalid"));
}

/// @dev This test checks that quorum will not be reached with <quorum valid signatures
function testFuzz_verifyCurrentQuorum_noQuorum(bytes memory encoded, uint256 numGuardians, uint256 numSigs) public {
vm.assume(encoded.length > 0);
numGuardians = bound(numGuardians, 1, type(uint8).max);
numSigs = bound(numSigs, 0, messages.quorum(numGuardians) - 1);

// Set the initial guardian set
address[] memory initialGuardians = new address[](numGuardians);

bytes32 message = keccak256(encoded);

Structs.Signature[] memory signatures = new Structs.Signature[](numSigs);

for (uint8 i = 0; i < numGuardians; ++i) {
(address addr, uint256 key) = makeAddrAndKey(string(abi.encode(i)));
initialGuardians[i] = addr;

if (i < numSigs) {
(signatures[i].v, signatures[i].r, signatures[i].s) = vm.sign(key, message);
signatures[i].guardianIndex = i;
}
}

// Create a guardian set
Structs.GuardianSet memory initialGuardianSet = Structs.GuardianSet({
keys: initialGuardians,
expirationTime: 0
});

messages.storeGuardianSetPub(initialGuardianSet, uint32(0));

(bool valid, string memory reason) = messages.verifyCurrentQuorum(message, signatures);
assertEq(valid, false);
assertEq(abi.encodePacked(reason), abi.encodePacked("no quorum"));
}

}
1 change: 1 addition & 0 deletions ethereum/foundry.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ remappings = [
'ds-test/=lib/forge-std/lib/ds-test/src/',
'forge-std/=lib/forge-std/src/',
'truffle/=node_modules/truffle/',
'wormhole-sdk/=lib/wormhole-solidity-sdk/src/'
]

[fmt]
Expand Down
7 changes: 7 additions & 0 deletions relayer/ethereum/forge-test/relayer/MockWormhole.sol
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,13 @@ contract MockWormhole is IWormhole {
revert("unsupported verifySignatures in wormhole mock");
}

function verifyCurrentQuorum(
bytes32, /*hash*/
Signature[] memory /*signatures*/
) external pure returns (bool /*valid*/, string memory /*reason*/) {
revert("unsupported verifyCurrentQuorum in wormhole mock");
}

function parseContractUpgrade(bytes memory /*encodedUpgrade*/ )
external
pure
Expand Down
Loading