certusone
/
wormhole
mirror of https://github.com/certusone/wormhole.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

ethereum: Simplified forked chain check 

No longer need to return uint16 max in chainId() for forked chains
This commit is contained in:
Kevin Peters 2022-09-09 04:26:15 +00:00 committed by kev1n-peters
parent 3f9c80a554
commit b1081d7d93
8 changed files with 20 additions and 78 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
ethereum/contracts/Getters.sol
View File

@ -27,9 +27,6 @@ contract Getters is State {
}
function chainId() public view returns (uint16) {
if (evmChainId() != block.chainid) {
return type(uint16).max;
}
return _state.provider.chainId;
}

13
ethereum/contracts/Governance.sol
View File

@ -25,6 +25,8 @@ abstract contract Governance is GovernanceStructs, Messages, Setters, ERC1967Upg
* @dev Upgrades a contract via Governance VAA/VM
*/
function submitContractUpgrade(bytes memory _vm) public {
require(evmChainId() == block.chainid, "bad fork");
Structs.VM memory vm = parseVM(_vm);
// Verify the VAA is valid before processing it
@ -37,8 +39,7 @@ abstract contract Governance is GovernanceStructs, Messages, Setters, ERC1967Upg
require(upgrade.module == module, "Invalid Module");
// Verify the VAA is for this chain
uint16 chainId = chainId();
require(upgrade.chain == chainId && chainId != type(uint16).max, "Invalid Chain");
require(upgrade.chain == chainId(), "Invalid Chain");
// Record the governance action as consumed
setGovernanceActionConsumed(vm.hash);
@ -63,7 +64,7 @@ abstract contract Governance is GovernanceStructs, Messages, Setters, ERC1967Upg
require(upgrade.module == module, "Invalid Module");
// Verify the VAA is for this chain
require(upgrade.chain == chainId(), "Invalid Chain");
require(upgrade.chain == chainId() && evmChainId() == block.chainid, "Invalid Chain");
// Record the governance action as consumed to prevent reentry
setGovernanceActionConsumed(vm.hash);
@ -88,7 +89,7 @@ abstract contract Governance is GovernanceStructs, Messages, Setters, ERC1967Upg
require(upgrade.module == module, "invalid Module");
// Verify the VAA is for this chain
require(upgrade.chain == chainId() || upgrade.chain == 0, "invalid Chain");
require((upgrade.chain == chainId() && evmChainId() == block.chainid) || upgrade.chain == 0, "invalid Chain");
// Verify the Guardian Set keys are not empty, this guards
// against the accidential upgrade to an empty GuardianSet
@ -127,7 +128,7 @@ abstract contract Governance is GovernanceStructs, Messages, Setters, ERC1967Upg
require(transfer.module == module, "invalid Module");
// Verify the VAA is for this chain
require(transfer.chain == chainId() || transfer.chain == 0, "invalid Chain");
require((transfer.chain == chainId() && evmChainId() == block.chainid) || transfer.chain == 0, "invalid Chain");
// Record the governance action as consumed to prevent reentry
setGovernanceActionConsumed(vm.hash);
@ -143,7 +144,7 @@ abstract contract Governance is GovernanceStructs, Messages, Setters, ERC1967Upg
* @dev Updates the `chainId` and `evmChainId` on a forked chain via Governance VAA/VM
*/
function submitRecoverChainId(bytes memory _vm) public {
require(chainId() == type(uint16).max, "invalid chain");
require(evmChainId() != block.chainid, "not a fork");
Structs.VM memory vm = parseVM(_vm);

3
ethereum/contracts/bridge/BridgeGetters.sol
View File

@ -27,9 +27,6 @@ contract BridgeGetters is BridgeState {
}
function chainId() public view returns (uint16){
if (evmChainId() != block.chainid) {
return type(uint16).max;
}
return _state.provider.chainId;
}

9
ethereum/contracts/bridge/BridgeGovernance.sol
View File

@ -33,7 +33,7 @@ contract BridgeGovernance is BridgeGetters, BridgeSetters, ERC1967Upgrade {
BridgeStructs.RegisterChain memory chain = parseRegisterChain(vm.payload);
require(chain.chainId == chainId() || chain.chainId == 0, "invalid chain id");
require((chain.chainId == chainId() && evmChainId() == block.chainid) || chain.chainId == 0, "invalid chain id");
require(bridgeContracts(chain.emitterChainID) == bytes32(0), "chain already registered");
setBridgeImplementation(chain.emitterChainID, chain.emitterAddress);
@ -41,6 +41,8 @@ contract BridgeGovernance is BridgeGetters, BridgeSetters, ERC1967Upgrade {
// Execute a UpgradeContract governance message
function upgrade(bytes memory encodedVM) public {
require(evmChainId() == block.chainid, "bad fork");
(IWormhole.VM memory vm, bool valid, string memory reason) = verifyGovernanceVM(encodedVM);
require(valid, reason);
@ -48,8 +50,7 @@ contract BridgeGovernance is BridgeGetters, BridgeSetters, ERC1967Upgrade {
BridgeStructs.UpgradeContract memory implementation = parseUpgrade(vm.payload);
uint16 chainId = chainId();
require(implementation.chainId == chainId && chainId != type(uint16).max, "wrong chain id");
require(implementation.chainId == chainId(), "wrong chain id");
upgradeImplementation(address(uint160(uint256(implementation.newContract))));
}
@ -58,7 +59,7 @@ contract BridgeGovernance is BridgeGetters, BridgeSetters, ERC1967Upgrade {
* @dev Updates the `chainId` and `evmChainId` on a forked chain via Governance VAA/VM
*/
function submitRecoverChainId(bytes memory encodedVM) public {
require(chainId() == type(uint16).max, "invalid chain");
require(evmChainId() != block.chainid, "not a fork");
(IWormhole.VM memory vm, bool valid, string memory reason) = verifyGovernanceVM(encodedVM);
require(valid, reason);

3
ethereum/contracts/nft/NFTBridgeGetters.sol
View File

@ -27,9 +27,6 @@ contract NFTBridgeGetters is NFTBridgeState {
}
function chainId() public view returns (uint16){
if (evmChainId() != block.chainid) {
return type(uint16).max;
}
return _state.provider.chainId;
}

9
ethereum/contracts/nft/NFTBridgeGovernance.sol
View File

@ -32,13 +32,15 @@ contract NFTBridgeGovernance is NFTBridgeGetters, NFTBridgeSetters, ERC1967Upgra
NFTBridgeStructs.RegisterChain memory chain = parseRegisterChain(vm.payload);
require(chain.chainId == chainId() || chain.chainId == 0, "invalid chain id");
require((chain.chainId == chainId() && evmChainId() == block.chainid) || chain.chainId == 0, "invalid chain id");
setBridgeImplementation(chain.emitterChainID, chain.emitterAddress);
}
// Execute a UpgradeContract governance message
function upgrade(bytes memory encodedVM) public {
require(evmChainId() == block.chainid, "bad fork");
(IWormhole.VM memory vm, bool valid, string memory reason) = verifyGovernanceVM(encodedVM);
require(valid, reason);
@ -46,8 +48,7 @@ contract NFTBridgeGovernance is NFTBridgeGetters, NFTBridgeSetters, ERC1967Upgra
NFTBridgeStructs.UpgradeContract memory implementation = parseUpgrade(vm.payload);
uint16 chainId = chainId();
require(implementation.chainId == chainId && chainId != type(uint16).max, "wrong chain id");
require(implementation.chainId == chainId(), "wrong chain id");
upgradeImplementation(address(uint160(uint256(implementation.newContract))));
}
@ -56,7 +57,7 @@ contract NFTBridgeGovernance is NFTBridgeGetters, NFTBridgeSetters, ERC1967Upgra
* @dev Updates the `chainId` and `evmChainId` on a forked chain via Governance VAA/VM
*/
function submitRecoverChainId(bytes memory encodedVM) public {
require(chainId() == type(uint16).max, "invalid chain");
require(evmChainId() != block.chainid, "not a fork");
(IWormhole.VM memory vm, bool valid, string memory reason) = verifyGovernanceVM(encodedVM);
require(valid, reason);

4
ethereum/forge-test/Bridge.t.sol
View File

@ -14,7 +14,7 @@ contract TestBridge is Bridge, Test {
require(converted == b, "truncate does not roundrip");
}
function testChainId() public {
function testEvmChainId() public {
vm.chainId(1);
setChainId(1);
setEvmChainId(1);
@ -23,8 +23,6 @@ contract TestBridge is Bridge, Test {
// fork occurs, block.chainid changes
vm.chainId(10001);
assertEq(chainId(), type(uint16).max);
assertEq(evmChainId(), 1);
setEvmChainId(10001);
assertEq(chainId(), 1);

54
ethereum/test/wormhole.js
View File

@ -614,57 +614,7 @@ contract("Wormhole", function () {
assert.ok(isUpgraded);
})
it("should revert smart contract upgrades with the bad fork chain ID (uint16 max)", async function () {
const initialized = new web3.eth.Contract(ImplementationFullABI, Wormhole.address);
const accounts = await web3.eth.getAccounts();
const mock = await MockImplementation.new();
const timestamp = 1000;
const nonce = 1001;
const emitterChainId = testGovernanceChainId;
const emitterAddress = testGovernanceContract
data = [
// Core
core,
// Action 1 (Contract Upgrade)
actionContractUpgrade,
// ChainID (max uint16 - bad fork)
web3.eth.abi.encodeParameter("uint16", 65535).substring(2 + (64 - 4)),
// New Contract Address
web3.eth.abi.encodeParameter("address", mock.address).substring(2),
].join('')
const vm = await signAndEncodeVM(
timestamp,
nonce,
emitterChainId,
emitterAddress,
0,
data,
[
testSigner1PK,
testSigner2PK,
testSigner3PK
],
1,
2
);
try {
await initialized.methods.submitContractUpgrade("0x" + vm).send({
value: 0,
from: accounts[0],
gasLimit: 1000000
});
assert.fail("contract upgrade for bad fork accepted")
} catch (e) {
assert.equal(e.data[Object.keys(e.data)[0]].reason, "Invalid Chain")
}
})
it("should revert recover chain ID governance packets on supported (non-bad-fork) chains", async function () {
it("should revert recover chain ID governance packets on canonical chains (non-forked)", async function () {
const initialized = new web3.eth.Contract(ImplementationFullABI, Wormhole.address);
const accounts = await web3.eth.getAccounts();
@ -708,7 +658,7 @@ contract("Wormhole", function () {
});
assert.fail("recover chain ID governance packet on supported chain accepted")
} catch (e) {
assert.equal(e.data[Object.keys(e.data)[0]].reason, "invalid chain")
assert.equal(e.data[Object.keys(e.data)[0]].reason, "not a fork")
}
})