From 1b1824378416d68da2dbf798a4a57809f98b9ed5 Mon Sep 17 00:00:00 2001 From: Michael Fletcher Date: Wed, 20 Nov 2024 17:20:10 +0000 Subject: [PATCH] Fix setConfig migration --- .../src/v0.8/llo-feeds/v0.5.0/Verifier.sol | 221 ++++------ .../llo-feeds/v0.5.0/interfaces/IVerifier.sol | 41 +- .../v0.5.0/test/gas/Gas_VerifierTest.t.sol | 10 +- .../v0.5.0/test/mocks/ErroredVerifier.sol | 19 +- .../test/verifier/BaseVerifierTest.t.sol | 104 ++--- .../test/verifier/VerifierSetConfigTest.t.sol | 377 ++++++++++++------ .../v0.5.0/test/verifier/VerifierTest.t.sol | 3 +- .../test/verifier/VerifierVerifyTest.t.sol | 87 +++- 8 files changed, 473 insertions(+), 389 deletions(-) diff --git a/contracts/src/v0.8/llo-feeds/v0.5.0/Verifier.sol b/contracts/src/v0.8/llo-feeds/v0.5.0/Verifier.sol index a3ca4019643..371aa3e9d30 100644 --- a/contracts/src/v0.8/llo-feeds/v0.5.0/Verifier.sol +++ b/contracts/src/v0.8/llo-feeds/v0.5.0/Verifier.sol @@ -37,24 +37,15 @@ contract Verifier is IVerifier, ConfirmedOwner, TypeAndVersionInterface { Role role; } - struct Config { - // Fault tolerance - uint8 f; - // Marks whether or not a configuration is active - bool isActive; - // Map of signer addresses to oracles - mapping(address => Signer) oracles; - } - struct VerifierState { - // The number of configs for this DON - uint32 configCount; // The block number of the block the last time the configuration was updated. uint32 latestConfigBlockNumber; // Whether the config is deactivated bool isActive; // Fault tolerance uint8 f; + // Number of signers + uint8 oracleCount; // Map of signer addresses to oracles mapping(address => Signer) oracles; } @@ -65,16 +56,16 @@ contract Verifier is IVerifier, ConfirmedOwner, TypeAndVersionInterface { /// @notice This event is emitted whenever a new DON configuration is set. event ConfigSet( - bytes32 indexed configId, - uint32 previousConfigBlockNumber, - bytes32 configDigest, - uint64 configCount, + bytes32 indexed configDigest, address[] signers, - bytes32[] offchainTransmitters, - uint8 f, - bytes onchainConfig, - uint64 offchainConfigVersion, - bytes offchainConfig + uint8 f + ); + + /// @notice This event is + event ConfigUpdated( + bytes32 indexed configDigest, + address[] prevSigners, + address[] newSigners ); /// @notice This event is emitted whenever a configuration is deactivated @@ -139,6 +130,9 @@ contract Verifier is IVerifier, ConfirmedOwner, TypeAndVersionInterface { /// @notice This error is thrown whenever a report fails to verify due to bad or duplicate signatures error BadVerification(); + /// @notice This error is thrown whenever a config digest is already set when setting the configuration + error ConfigDigestAlreadySet(); + /// @notice The address of the verifier proxy address private immutable i_verifierProxyAddr; @@ -255,78 +249,64 @@ contract Verifier is IVerifier, ConfirmedOwner, TypeAndVersionInterface { } /// @inheritdoc IVerifier - function setConfigFromSource( - bytes32 configId, - uint256 sourceChainId, - address sourceAddress, - uint32 configCount, - address[] memory signers, - bytes32[] memory offchainTransmitters, + function updateConfig( + bytes32 configDigest, + address[] calldata prevSigners, + address[] calldata newSigners, + uint8 f + ) external override checkConfigValid(newSigners.length, f) onlyOwner { + VerifierState storage config = s_verifierStates[configDigest]; + + if (config.f == 0) revert DigestNotSet(configDigest); + + // We must be removing the number of signers that were originally set + if(config.oracleCount != prevSigners.length){ + revert NonUniqueSignatures(); + } + + for (uint256 i; i < prevSigners.length; ++i) { + // Check the signers being removed are not zero address or duplicates + if(config.oracles[prevSigners[i]].role == Role.Unset){ + revert NonUniqueSignatures(); + } + + delete config.oracles[prevSigners[i]]; + } + + // Once signers have been cleared we can set the new signers + _setConfig(configDigest, newSigners, f, new Common.AddressAndWeight[](0), true); + + + emit ConfigUpdated(configDigest, prevSigners, newSigners); + } + + /// @inheritdoc IVerifier + function setConfig( + bytes32 configDigest, + address[] calldata signers, uint8 f, - bytes memory onchainConfig, - uint64 offchainConfigVersion, - bytes memory offchainConfig, Common.AddressAndWeight[] memory recipientAddressesAndWeights ) external override checkConfigValid(signers.length, f) onlyOwner { - _setConfig( - configId, - sourceChainId, - sourceAddress, - configCount, - signers, - offchainTransmitters, - f, - onchainConfig, - offchainConfigVersion, - offchainConfig, - recipientAddressesAndWeights - ); + _setConfig(configDigest, signers, f, recipientAddressesAndWeights, false); } - /// @notice Sets config based on the given arguments - /// @param configId Config ID to set config for - /// @param sourceChainId Chain ID of source config - /// @param sourceAddress Address of source config Verifier - /// @param configCount The number of times a new configuration has been set - /// @param signers addresses with which oracles sign the reports - /// @param offchainTransmitters CSA key for the ith Oracle - /// @param f number of faulty oracles the system can tolerate - /// @param onchainConfig serialized configuration used by the contract (and possibly oracles) - /// @param offchainConfigVersion version number for offchainEncoding schema - /// @param offchainConfig serialized configuration used by the oracles exclusively and only passed through the contract - /// @param recipientAddressesAndWeights the addresses and weights of all the recipients to receive rewards function _setConfig( - bytes32 configId, - uint256 sourceChainId, - address sourceAddress, - uint32 configCount, - address[] memory signers, - bytes32[] memory offchainTransmitters, + bytes32 configDigest, + address[] calldata signers, uint8 f, - bytes memory onchainConfig, - uint64 offchainConfigVersion, - bytes memory offchainConfig, - Common.AddressAndWeight[] memory recipientAddressesAndWeights + Common.AddressAndWeight[] memory recipientAddressesAndWeights, + bool updateConfig ) internal { - bytes32 configDigest = _configDigestFromConfigData( - configId, - sourceChainId, - sourceAddress, - configCount, - signers, - offchainTransmitters, - f, - onchainConfig, - offchainConfigVersion, - offchainConfig - ); - VerifierState storage verifierState = s_verifierStates[configDigest]; + if(verifierState.f > 0 && !updateConfig) { + revert ConfigDigestAlreadySet(); + } + verifierState.latestConfigBlockNumber = uint32(block.number); - verifierState.configCount = configCount; verifierState.f = f; verifierState.isActive = true; + verifierState.oracleCount = uint8(signers.length); for (uint8 i; i < signers.length; ++i) { address signerAddr = signers[i]; @@ -345,77 +325,19 @@ contract Verifier is IVerifier, ConfirmedOwner, TypeAndVersionInterface { }); } - IVerifierProxy(i_verifierProxyAddr).setVerifier( - bytes32(0), - configDigest, - recipientAddressesAndWeights - ); - - emit ConfigSet( - configId, - 0, - configDigest, - configCount, - signers, - offchainTransmitters, - f, - onchainConfig, - offchainConfigVersion, - offchainConfig - ); - } - - /// @notice Generates the config digest from config data - /// @param configId to set config for - /// @param sourceChainId Chain ID of source config - /// @param sourceAddress Address of source config Verifier - /// @param configCount ordinal number of this config setting among all config settings over the life of this contract - /// @param signers ith element is address ith oracle uses to sign a report - /// @param offchainTransmitters ith element is address ith oracle used to transmit reports (in this case used for flexible additional field, such as CSA pub keys) - /// @param f maximum number of faulty/dishonest oracles the protocol can tolerate while still working correctly - /// @param onchainConfig serialized configuration used by the contract (and possibly oracles) - /// @param offchainConfigVersion version of the serialization format used for "offchainConfig" parameter - /// @param offchainConfig serialized configuration used by the oracles exclusively and only passed through the contract - /// @dev This function is a modified version of the method from OCR2Abstract - function _configDigestFromConfigData( - bytes32 configId, - uint256 sourceChainId, - address sourceAddress, - uint64 configCount, - address[] memory signers, - bytes32[] memory offchainTransmitters, - uint8 f, - bytes memory onchainConfig, - uint64 offchainConfigVersion, - bytes memory offchainConfig - ) internal pure returns (bytes32) { - - bytes[] memory signersAsBytes = new bytes[](signers.length); - for (uint i; i < signers.length; ++i){ - signersAsBytes[i] = abi.encodePacked(signers[i]); - } - - uint256 h = uint256( - keccak256( - abi.encode( - configId, - sourceChainId, - sourceAddress, - configCount, - signersAsBytes, - offchainTransmitters, - f, - onchainConfig, - offchainConfigVersion, - offchainConfig - ) - ) + if(!updateConfig) { + IVerifierProxy(i_verifierProxyAddr).setVerifier( + bytes32(0), + configDigest, + recipientAddressesAndWeights ); - uint256 prefixMask = type(uint256).max << (256 - 16); // 0xFFFF00..00 - // 0x0009 corresponds to ConfigDigestPrefixMercuryV02 in libocr - uint256 prefix = 0x0009 << (256 - 16); // 0x000900..00 - return bytes32((prefix & prefixMask) | (h & ~prefixMask)); + emit ConfigSet( + configDigest, + signers, + f + ); + } } /// @inheritdoc IVerifier @@ -441,10 +363,9 @@ contract Verifier is IVerifier, ConfirmedOwner, TypeAndVersionInterface { /// @inheritdoc IVerifier function latestConfigDetails( bytes32 configDigest - ) external view override returns (uint32 configCount, uint32 blockNumber) { + ) external view override returns (uint32 blockNumber) { VerifierState storage verifierState = s_verifierStates[configDigest]; return ( - verifierState.configCount, verifierState.latestConfigBlockNumber ); } diff --git a/contracts/src/v0.8/llo-feeds/v0.5.0/interfaces/IVerifier.sol b/contracts/src/v0.8/llo-feeds/v0.5.0/interfaces/IVerifier.sol index f57f4b9bc93..304ef31b94f 100644 --- a/contracts/src/v0.8/llo-feeds/v0.5.0/interfaces/IVerifier.sol +++ b/contracts/src/v0.8/llo-feeds/v0.5.0/interfaces/IVerifier.sol @@ -19,33 +19,33 @@ interface IVerifier is IERC165 { function verify(bytes calldata signedReport, address sender) external returns (bytes memory verifierResponse); /** - * @notice identical to `setConfig` except with args for sourceChainId and sourceAddress - * @param configId Config ID to set config for - * @param sourceChainId Chain ID of source config - * @param sourceAddress Address of source config Verifier - * @param configCount The config count for the configuration + * @notice sets a configuration and its associated keys and f + * @param configDigest The digest of the configuration we're setting * @param signers addresses with which oracles sign the reports - * @param offchainTransmitters CSA key for the ith Oracle * @param f number of faulty oracles the system can tolerate - * @param onchainConfig serialized configuration used by the contract (and possibly oracles) - * @param offchainConfigVersion version number for offchainEncoding schema - * @param offchainConfig serialized configuration used by the oracles exclusively and only passed through the contract * @param recipientAddressesAndWeights the addresses and weights of all the recipients to receive rewards */ - function setConfigFromSource( - bytes32 configId, - uint256 sourceChainId, - address sourceAddress, - uint32 configCount, - address[] memory signers, - bytes32[] memory offchainTransmitters, + function setConfig( + bytes32 configDigest, + address[] calldata signers, uint8 f, - bytes memory onchainConfig, - uint64 offchainConfigVersion, - bytes memory offchainConfig, Common.AddressAndWeight[] memory recipientAddressesAndWeights ) external; + /** + * @notice updates a configuration that has been set + * @param configDigest The digest of the configuration we're updating + * @param prevSigners the existing signers that need to be removed + * @param newSigners the signers to be added + * @param f the newnumber of faulty oracles the system can tolerate + */ + function updateConfig( + bytes32 configDigest, + address[] calldata prevSigners, + address[] calldata newSigners, + uint8 f + ) external; + /** * @notice Activates the configuration for a config digest * @param configDigest The config digest to activate @@ -63,10 +63,9 @@ interface IVerifier is IERC165 { /** * @notice information about current offchain reporting protocol configuration * @param configDigest Config Digest to fetch data for - * @return configCount ordinal number of current config, out of all configs applied to this contract so far * @return blockNumber block at which this config was set */ function latestConfigDetails( bytes32 configDigest - ) external view returns (uint32 configCount, uint32 blockNumber); + ) external view returns (uint32 blockNumber); } diff --git a/contracts/src/v0.8/llo-feeds/v0.5.0/test/gas/Gas_VerifierTest.t.sol b/contracts/src/v0.8/llo-feeds/v0.5.0/test/gas/Gas_VerifierTest.t.sol index 00e65618968..bbbda4f619f 100644 --- a/contracts/src/v0.8/llo-feeds/v0.5.0/test/gas/Gas_VerifierTest.t.sol +++ b/contracts/src/v0.8/llo-feeds/v0.5.0/test/gas/Gas_VerifierTest.t.sol @@ -17,7 +17,7 @@ contract Verifier_setConfig is BaseTest { } function testSetConfigSuccess_gas() public { - s_verifier.setConfigFromSource( + bytes32 configDigest = _configDigestFromConfigData( FEED_ID, SOURCE_CHAIN_ID, SOURCE_ADDRESS, @@ -27,7 +27,13 @@ contract Verifier_setConfig is BaseTest { FAULT_TOLERANCE, bytes(""), 1, - bytes(""), + bytes("") + ); + + s_verifier.setConfig( + configDigest, + s_signerAddrs, + FAULT_TOLERANCE, new Common.AddressAndWeight[](0) ); } diff --git a/contracts/src/v0.8/llo-feeds/v0.5.0/test/mocks/ErroredVerifier.sol b/contracts/src/v0.8/llo-feeds/v0.5.0/test/mocks/ErroredVerifier.sol index 67b9e0cefd8..c92bcb1ac27 100644 --- a/contracts/src/v0.8/llo-feeds/v0.5.0/test/mocks/ErroredVerifier.sol +++ b/contracts/src/v0.8/llo-feeds/v0.5.0/test/mocks/ErroredVerifier.sol @@ -13,6 +13,7 @@ contract ErroredVerifier is IVerifier { error FailedToVerify(); error FailedToSetConfig(); + error FailedToUnsetConfig(); error FailedToActivateConfig(); error FailedToDeactivateConfig(); error FailedToActivateFeed(); @@ -40,19 +41,15 @@ contract ErroredVerifier is IVerifier { revert FailedToVerify(); } + function updateConfig(bytes32, address[] calldata, address[] calldata, uint8) external pure { + revert FailedToUnsetConfig(); + } - function setConfigFromSource( + function setConfig( bytes32, - uint256, - address, - uint32, - address[] memory, - bytes32[] memory, + address[] calldata, uint8, - bytes memory, - uint64, - bytes memory, - Common.AddressAndWeight[] memory + Common.AddressAndWeight[] calldata ) external pure override { revert FailedToSetConfig(); } @@ -65,7 +62,7 @@ contract ErroredVerifier is IVerifier { revert FailedToDeactivateConfig(); } - function latestConfigDetails(bytes32) external pure override returns (uint32, uint32) { + function latestConfigDetails(bytes32) external pure override returns (uint32) { revert FailedToGetLatestConfigDetails(); } } diff --git a/contracts/src/v0.8/llo-feeds/v0.5.0/test/verifier/BaseVerifierTest.t.sol b/contracts/src/v0.8/llo-feeds/v0.5.0/test/verifier/BaseVerifierTest.t.sol index 9b24563fa66..db220f6547f 100644 --- a/contracts/src/v0.8/llo-feeds/v0.5.0/test/verifier/BaseVerifierTest.t.sol +++ b/contracts/src/v0.8/llo-feeds/v0.5.0/test/verifier/BaseVerifierTest.t.sol @@ -296,20 +296,7 @@ contract BaseTestWithConfiguredVerifierAndFeeManager is BaseTest { Signer[] memory signers = _getSigners(MAX_ORACLES); s_verifierProxy.initializeVerifier(address(s_verifier)); - s_verifier.setConfigFromSource( - FEED_ID, - SOURCE_CHAIN_ID, - SOURCE_ADDRESS, - 1, - _getSignerAddresses(signers), - s_offchaintransmitters, - FAULT_TOLERANCE, - bytes(""), - VERIFIER_VERSION, - bytes(""), - new Common.AddressAndWeight[](0) - ); - + v1ConfigDigest = _configDigestFromConfigData( FEED_ID, SOURCE_CHAIN_ID, @@ -323,20 +310,13 @@ contract BaseTestWithConfiguredVerifierAndFeeManager is BaseTest { bytes("") ); - s_verifier.setConfigFromSource( - FEED_ID_V3, - SOURCE_CHAIN_ID, - SOURCE_ADDRESS, - 1, + s_verifier.setConfig( + v1ConfigDigest, _getSignerAddresses(signers), - s_offchaintransmitters, FAULT_TOLERANCE, - bytes(""), - VERIFIER_VERSION, - bytes(""), new Common.AddressAndWeight[](0) ); - + v3ConfigDigest = _configDigestFromConfigData( FEED_ID_V3, SOURCE_CHAIN_ID, @@ -350,6 +330,13 @@ contract BaseTestWithConfiguredVerifierAndFeeManager is BaseTest { bytes("") ); + s_verifier.setConfig( + v3ConfigDigest, + _getSignerAddresses(signers), + FAULT_TOLERANCE, + new Common.AddressAndWeight[](0) + ); + link = new ERC20Mock("LINK", "LINK", ADMIN, 0); native = new WERC20Mock(); @@ -486,7 +473,7 @@ contract BaseTestWithMultipleConfiguredDigests is BaseTestWithConfiguredVerifier // Verifier 1, Feed 1, Config 2 Signer[] memory secondSetOfSigners = _getSigners(8); - s_verifier.setConfigFromSource( + s_configDigestTwo = _configDigestFromConfigData( FEED_ID, SOURCE_CHAIN_ID, SOURCE_ADDRESS, @@ -496,26 +483,18 @@ contract BaseTestWithMultipleConfiguredDigests is BaseTestWithConfiguredVerifier FAULT_TOLERANCE_TWO, bytes(""), 2, - bytes(""), - new Common.AddressAndWeight[](0) + bytes("") ); - - s_configDigestTwo = _configDigestFromConfigData( - FEED_ID, - SOURCE_CHAIN_ID, - SOURCE_ADDRESS, - 2, + s_verifier.setConfig( + s_configDigestTwo, _getSignerAddresses(secondSetOfSigners), - s_offchaintransmitters, FAULT_TOLERANCE_TWO, - bytes(""), - 2, - bytes("") + new Common.AddressAndWeight[](0) ); // Verifier 1, Feed 1, Config 3 Signer[] memory thirdSetOfSigners = _getSigners(5); - s_verifier.setConfigFromSource( + s_configDigestThree = _configDigestFromConfigData( FEED_ID, SOURCE_CHAIN_ID, SOURCE_ADDRESS, @@ -525,24 +504,17 @@ contract BaseTestWithMultipleConfiguredDigests is BaseTestWithConfiguredVerifier FAULT_TOLERANCE_THREE, bytes(""), 3, - bytes(""), - new Common.AddressAndWeight[](0) + bytes("") ); - s_configDigestThree = _configDigestFromConfigData( - FEED_ID, - SOURCE_CHAIN_ID, - SOURCE_ADDRESS, - 3, + s_verifier.setConfig( + s_configDigestThree, _getSignerAddresses(thirdSetOfSigners), - s_offchaintransmitters, FAULT_TOLERANCE_THREE, - bytes(""), - 3, - bytes("") + new Common.AddressAndWeight[](0) ); // Verifier 1, Feed 2, Config 1 - s_verifier.setConfigFromSource( + s_configDigestFour = _configDigestFromConfigData( FEED_ID_2, SOURCE_CHAIN_ID, SOURCE_ADDRESS, @@ -552,25 +524,18 @@ contract BaseTestWithMultipleConfiguredDigests is BaseTestWithConfiguredVerifier FAULT_TOLERANCE, bytes(""), 4, - bytes(""), - new Common.AddressAndWeight[](0) + bytes("") ); - s_configDigestFour = _configDigestFromConfigData( - FEED_ID_2, - SOURCE_CHAIN_ID, - SOURCE_ADDRESS, - 1, + s_verifier.setConfig( + s_configDigestFour, _getSignerAddresses(signers), - s_offchaintransmitters, FAULT_TOLERANCE, - bytes(""), - 4, - bytes("") + new Common.AddressAndWeight[](0) ); // Verifier 2, Feed 3, Config 1 s_verifierProxy.initializeVerifier(address(s_verifier_2)); - s_verifier_2.setConfigFromSource( + s_configDigestFive = _configDigestFromConfigData( FEED_ID_3, SOURCE_CHAIN_ID, SOURCE_ADDRESS, @@ -580,20 +545,13 @@ contract BaseTestWithMultipleConfiguredDigests is BaseTestWithConfiguredVerifier FAULT_TOLERANCE, bytes(""), 5, - bytes(""), - new Common.AddressAndWeight[](0) + bytes("") ); - s_configDigestFive = _configDigestFromConfigData( - FEED_ID_3, - SOURCE_CHAIN_ID, - SOURCE_ADDRESS, - 1, + s_verifier_2.setConfig( + s_configDigestFive, _getSignerAddresses(signers), - s_offchaintransmitters, FAULT_TOLERANCE, - bytes(""), - 5, - bytes("") + new Common.AddressAndWeight[](0) ); } } diff --git a/contracts/src/v0.8/llo-feeds/v0.5.0/test/verifier/VerifierSetConfigTest.t.sol b/contracts/src/v0.8/llo-feeds/v0.5.0/test/verifier/VerifierSetConfigTest.t.sol index ec88410d3ac..9276450e133 100644 --- a/contracts/src/v0.8/llo-feeds/v0.5.0/test/verifier/VerifierSetConfigTest.t.sol +++ b/contracts/src/v0.8/llo-feeds/v0.5.0/test/verifier/VerifierSetConfigTest.t.sol @@ -15,8 +15,7 @@ contract VerifierSetConfigTest is BaseTest { vm.expectRevert("Only callable by owner"); Signer[] memory signers = _getSigners(MAX_ORACLES); - changePrank(USER); - s_verifier.setConfigFromSource( + bytes32 configDigest = _configDigestFromConfigData( FEED_ID, SOURCE_CHAIN_ID, SOURCE_ADDRESS, @@ -26,15 +25,22 @@ contract VerifierSetConfigTest is BaseTest { FAULT_TOLERANCE, bytes(""), VERIFIER_VERSION, - bytes(""), + bytes("") + ); + + changePrank(USER); + s_verifier.setConfig( + configDigest, + _getSignerAddresses(signers), + FAULT_TOLERANCE, new Common.AddressAndWeight[](0) ); } function test_revertsIfSetWithTooManySigners() public { address[] memory signers = new address[](MAX_ORACLES + 1); - vm.expectRevert(abi.encodeWithSelector(Verifier.ExcessSigners.selector, signers.length, MAX_ORACLES)); - s_verifier.setConfigFromSource( + + bytes32 configDigest = _configDigestFromConfigData( FEED_ID, SOURCE_CHAIN_ID, SOURCE_ADDRESS, @@ -44,15 +50,22 @@ contract VerifierSetConfigTest is BaseTest { FAULT_TOLERANCE, bytes(""), VERIFIER_VERSION, - bytes(""), + bytes("") + ); + + vm.expectRevert(abi.encodeWithSelector(Verifier.ExcessSigners.selector, signers.length, MAX_ORACLES)); + s_verifier.setConfig( + configDigest, + signers, + FAULT_TOLERANCE, new Common.AddressAndWeight[](0) ); } function test_revertsIfFaultToleranceIsZero() public { - vm.expectRevert(abi.encodeWithSelector(Verifier.FaultToleranceMustBePositive.selector)); Signer[] memory signers = _getSigners(MAX_ORACLES); - s_verifier.setConfigFromSource( + + bytes32 configDigest = _configDigestFromConfigData( FEED_ID, SOURCE_CHAIN_ID, SOURCE_ADDRESS, @@ -62,7 +75,14 @@ contract VerifierSetConfigTest is BaseTest { 0, bytes(""), VERIFIER_VERSION, - bytes(""), + bytes("") + ); + + vm.expectRevert(abi.encodeWithSelector(Verifier.FaultToleranceMustBePositive.selector)); + s_verifier.setConfig( + configDigest, + _getSignerAddresses(signers), + 0, new Common.AddressAndWeight[](0) ); } @@ -72,10 +92,7 @@ contract VerifierSetConfigTest is BaseTest { signers[0] = address(1000); signers[1] = address(1001); - vm.expectRevert( - abi.encodeWithSelector(Verifier.InsufficientSigners.selector, signers.length, FAULT_TOLERANCE * 3 + 1) - ); - s_verifier.setConfigFromSource( + bytes32 configDigest = _configDigestFromConfigData( FEED_ID, SOURCE_CHAIN_ID, SOURCE_ADDRESS, @@ -85,7 +102,16 @@ contract VerifierSetConfigTest is BaseTest { FAULT_TOLERANCE, bytes(""), VERIFIER_VERSION, - bytes(""), + bytes("") + ); + + vm.expectRevert( + abi.encodeWithSelector(Verifier.InsufficientSigners.selector, signers.length, FAULT_TOLERANCE * 3 + 1) + ); + s_verifier.setConfig( + configDigest, + signers, + FAULT_TOLERANCE, new Common.AddressAndWeight[](0) ); } @@ -94,8 +120,8 @@ contract VerifierSetConfigTest is BaseTest { Signer[] memory signers = _getSigners(MAX_ORACLES); address[] memory signerAddrs = _getSignerAddresses(signers); signerAddrs[0] = signerAddrs[1]; - vm.expectRevert(abi.encodeWithSelector(Verifier.NonUniqueSignatures.selector)); - s_verifier.setConfigFromSource( + + bytes32 configDigest = _configDigestFromConfigData( FEED_ID, SOURCE_CHAIN_ID, SOURCE_ADDRESS, @@ -105,7 +131,14 @@ contract VerifierSetConfigTest is BaseTest { FAULT_TOLERANCE, bytes(""), VERIFIER_VERSION, - bytes(""), + bytes("") + ); + + vm.expectRevert(abi.encodeWithSelector(Verifier.NonUniqueSignatures.selector)); + s_verifier.setConfig( + configDigest, + signerAddrs, + FAULT_TOLERANCE, new Common.AddressAndWeight[](0) ); } @@ -114,8 +147,8 @@ contract VerifierSetConfigTest is BaseTest { Signer[] memory signers = _getSigners(MAX_ORACLES); address[] memory signerAddrs = _getSignerAddresses(signers); signerAddrs[0] = address(0); - vm.expectRevert(abi.encodeWithSelector(Verifier.ZeroAddress.selector)); - s_verifier.setConfigFromSource( + + bytes32 configDigest = _configDigestFromConfigData( FEED_ID, SOURCE_CHAIN_ID, SOURCE_ADDRESS, @@ -125,7 +158,14 @@ contract VerifierSetConfigTest is BaseTest { FAULT_TOLERANCE, bytes(""), VERIFIER_VERSION, - bytes(""), + bytes("") + ); + + vm.expectRevert(abi.encodeWithSelector(Verifier.ZeroAddress.selector)); + s_verifier.setConfig( + configDigest, + signerAddrs, + FAULT_TOLERANCE, new Common.AddressAndWeight[](0) ); } @@ -134,7 +174,8 @@ contract VerifierSetConfigTest is BaseTest { Signer[] memory signers = _getSigners(MAX_ORACLES); s_verifierProxy.initializeVerifier(address(s_verifier)); - s_verifier.setConfigFromSource( + + bytes32 configDigest = _configDigestFromConfigData( FEED_ID, SOURCE_CHAIN_ID, SOURCE_ADDRESS, @@ -144,34 +185,159 @@ contract VerifierSetConfigTest is BaseTest { FAULT_TOLERANCE, bytes(""), VERIFIER_VERSION, - bytes(""), - new Common.AddressAndWeight[](0) + bytes("") ); - bytes32 configDigest = _configDigestFromConfigData( - FEED_ID, - SOURCE_CHAIN_ID, - SOURCE_ADDRESS, - 1, + s_verifier.setConfig( + configDigest, _getSignerAddresses(signers), - s_offchaintransmitters, FAULT_TOLERANCE, - bytes(""), - VERIFIER_VERSION, - bytes("") + new Common.AddressAndWeight[](0) + ); + + uint32 blockNumber = s_verifier.latestConfigDetails(configDigest); + assertEq(blockNumber, block.number); + } +} + +contract VerifierUpdateConfigTest is BaseTest { + + function setUp() public virtual override { + BaseTest.setUp(); + + s_verifierProxy.initializeVerifier(address(s_verifier)); + } + + function test_updateConfig() public { + // Get initial signers and config digest + address[] memory signerAddresses = _getSignerAddresses(_getSigners(15)); + bytes32 configDigest = keccak256("test222"); + + // Set initial config + s_verifier.setConfig( + configDigest, + signerAddresses, + 4, + new Common.AddressAndWeight[](0) + ); + + // Unset the config + s_verifier.updateConfig(configDigest, signerAddresses, signerAddresses, 4); + } + + function test_updateConfigRevertsIfFIsZero() public { + // Get initial signers and config digest + address[] memory signerAddresses = _getSignerAddresses(_getSigners(15)); + bytes32 configDigest = keccak256("test"); + + // Set initial config + s_verifier.setConfig( + configDigest, + signerAddresses, + 4, + new Common.AddressAndWeight[](0) ); - (uint32 configCount, uint32 blockNumber) = s_verifier.latestConfigDetails(configDigest); - assertEq(configCount, 1); + // Try to update with f=0 + vm.expectRevert(Verifier.FaultToleranceMustBePositive.selector); + s_verifier.updateConfig(configDigest, signerAddresses, signerAddresses, 0); + } + + function test_updateConfigRevertsIfFTooHigh() public { + // Get initial signers and config digest + address[] memory signerAddresses = _getSignerAddresses(_getSigners(15)); + bytes32 configDigest = keccak256("test"); + + // Set initial config + s_verifier.setConfig( + configDigest, + signerAddresses, + 4, + new Common.AddressAndWeight[](0) + ); + + // Try to update with f too high + vm.expectRevert(abi.encodeWithSelector(Verifier.InsufficientSigners.selector, signerAddresses.length, 46)); + s_verifier.updateConfig(configDigest, signerAddresses, signerAddresses, 15); + } + + function test_updateConfigWithDifferentSigners() public { + // Get initial signers and config digest + address[] memory initialSigners = _getSignerAddresses(_getSigners(15)); + bytes32 configDigest = keccak256("test"); + + // Set initial config + s_verifier.setConfig( + configDigest, + initialSigners, + 4, + new Common.AddressAndWeight[](0) + ); + + // Get new signers + address[] memory newSigners = _getSignerAddresses(_getSigners(20)); + + // Update config with new signers + s_verifier.updateConfig(configDigest, initialSigners, newSigners, 6); + + // Verify config was updated + uint32 blockNumber = s_verifier.latestConfigDetails(configDigest); assertEq(blockNumber, block.number); } + + function test_updateConfigRevertsIfDigestNotSet() public { + address[] memory signerAddresses = _getSignerAddresses(_getSigners(15)); + bytes32 nonExistentDigest = keccak256("nonexistent"); + + vm.expectRevert(abi.encodeWithSelector(Verifier.DigestNotSet.selector, nonExistentDigest)); + s_verifier.updateConfig(nonExistentDigest, signerAddresses, signerAddresses, 4); + } + + function test_updateConfigRevertsIfPrevSignersLengthMismatch() public { + // Get initial signers and config digest + address[] memory initialSigners = _getSignerAddresses(_getSigners(15)); + bytes32 configDigest = keccak256("test"); + + // Set initial config + s_verifier.setConfig( + configDigest, + initialSigners, + 4, + new Common.AddressAndWeight[](0) + ); + + // Try to update with wrong number of previous signers + address[] memory wrongPrevSigners = _getSignerAddresses(_getSigners(10)); + address[] memory newSigners = _getSignerAddresses(_getSigners(15)); + + vm.expectRevert(Verifier.NonUniqueSignatures.selector); + s_verifier.updateConfig(configDigest, wrongPrevSigners, newSigners, 4); + } + + function test_updateConfigRevertsIfCalledByNonOwner() public { + address[] memory signerAddresses = _getSignerAddresses(_getSigners(15)); + bytes32 configDigest = keccak256("test"); + + // Set initial config + s_verifier.setConfig( + configDigest, + signerAddresses, + 4, + new Common.AddressAndWeight[](0) + ); + + // Try to update as non-owner + changePrank(USER); + vm.expectRevert("Only callable by owner"); + s_verifier.updateConfig(configDigest, signerAddresses, signerAddresses, 4); + } } contract VerifierSetConfigWhenThereAreMultipleDigestsTest is BaseTestWithMultipleConfiguredDigests { function test_correctlyUpdatesTheDigestInTheProxy() public { Signer[] memory newSigners = _getSigners(15); - s_verifier.setConfigFromSource( + bytes32 configDigest = _configDigestFromConfigData( FEED_ID, SOURCE_CHAIN_ID, SOURCE_ADDRESS, @@ -181,21 +347,14 @@ contract VerifierSetConfigWhenThereAreMultipleDigestsTest is BaseTestWithMultipl 4, bytes(""), VERIFIER_VERSION, - bytes(""), - new Common.AddressAndWeight[](0) + bytes("") ); - bytes32 configDigest = _configDigestFromConfigData( - FEED_ID, - SOURCE_CHAIN_ID, - SOURCE_ADDRESS, - 1, + s_verifier.setConfig( + configDigest, _getSignerAddresses(newSigners), - s_offchaintransmitters, 4, - bytes(""), - VERIFIER_VERSION, - bytes("") + new Common.AddressAndWeight[](0) ); address verifierAddr = s_verifierProxy.getVerifier(configDigest); @@ -205,7 +364,7 @@ contract VerifierSetConfigWhenThereAreMultipleDigestsTest is BaseTestWithMultipl function test_correctlyUpdatesDigestsOnMultipleVerifiersInTheProxy() public { Signer[] memory newSigners = _getSigners(15); - s_verifier.setConfigFromSource( + bytes32 configDigest = _configDigestFromConfigData( FEED_ID_2, SOURCE_CHAIN_ID, SOURCE_ADDRESS, @@ -215,27 +374,20 @@ contract VerifierSetConfigWhenThereAreMultipleDigestsTest is BaseTestWithMultipl 4, bytes(""), VERIFIER_VERSION, - bytes(""), - new Common.AddressAndWeight[](0) + bytes("") ); - bytes32 configDigest = _configDigestFromConfigData( - FEED_ID_2, - SOURCE_CHAIN_ID, - SOURCE_ADDRESS, - 1, + s_verifier.setConfig( + configDigest, _getSignerAddresses(newSigners), - s_offchaintransmitters, 4, - bytes(""), - VERIFIER_VERSION, - bytes("") + new Common.AddressAndWeight[](0) ); address verifierAddr = s_verifierProxy.getVerifier(configDigest); assertEq(verifierAddr, address(s_verifier)); - s_verifier_2.setConfigFromSource( + bytes32 configDigest2 = _configDigestFromConfigData( FEED_ID_3, SOURCE_CHAIN_ID, SOURCE_ADDRESS, @@ -245,21 +397,14 @@ contract VerifierSetConfigWhenThereAreMultipleDigestsTest is BaseTestWithMultipl 4, bytes(""), VERIFIER_VERSION, - bytes(""), - new Common.AddressAndWeight[](0) + bytes("") ); - bytes32 configDigest2 = _configDigestFromConfigData( - FEED_ID_3, - SOURCE_CHAIN_ID, - SOURCE_ADDRESS, - 1, + s_verifier_2.setConfig( + configDigest2, _getSignerAddresses(newSigners), - s_offchaintransmitters, 4, - bytes(""), - VERIFIER_VERSION, - bytes("") + new Common.AddressAndWeight[](0) ); address verifierAddr2 = s_verifierProxy.getVerifier(configDigest2); @@ -271,21 +416,7 @@ contract VerifierSetConfigWhenThereAreMultipleDigestsTest is BaseTestWithMultipl Signer[] memory newSigners = _getSigners(15); - s_verifier.setConfigFromSource( - FEED_ID, - SOURCE_CHAIN_ID, - SOURCE_ADDRESS, - 1, - _getSignerAddresses(newSigners), - s_offchaintransmitters, - 4, - bytes(""), - VERIFIER_VERSION, - bytes(""), - new Common.AddressAndWeight[](0) - ); - - bytes32 expectedConfigDigest = _configDigestFromConfigData( + bytes32 configDigest = _configDigestFromConfigData( FEED_ID, SOURCE_CHAIN_ID, SOURCE_ADDRESS, @@ -298,31 +429,23 @@ contract VerifierSetConfigWhenThereAreMultipleDigestsTest is BaseTestWithMultipl bytes("") ); - bytes32 configDigest = _configDigestFromConfigData( - FEED_ID, - SOURCE_CHAIN_ID, - SOURCE_ADDRESS, - 1, + s_verifier.setConfig( + configDigest, _getSignerAddresses(newSigners), - s_offchaintransmitters, 4, - bytes(""), - VERIFIER_VERSION, - bytes("") + new Common.AddressAndWeight[](0) ); - (uint32 configCount, uint32 blockNumber) = s_verifier.latestConfigDetails(configDigest); + uint32 blockNumber = s_verifier.latestConfigDetails(configDigest); - assertEq(configCount, s_numConfigsSet + 1); assertEq(blockNumber, block.number); - assertEq(configDigest, expectedConfigDigest); } function test_revertsIfDuplicateConfigIsSet() public { // Set initial config - s_verifier.setConfigFromSource( + bytes32 configDigest = _configDigestFromConfigData( FEED_ID, - SOURCE_CHAIN_ID, + SOURCE_CHAIN_ID, SOURCE_ADDRESS, 1, _getSignerAddresses(_getSigners(15)), @@ -330,32 +453,31 @@ contract VerifierSetConfigWhenThereAreMultipleDigestsTest is BaseTestWithMultipl 4, bytes(""), VERIFIER_VERSION, - bytes(""), + bytes("") + ); + + s_verifier.setConfig( + configDigest, + _getSignerAddresses(_getSigners(15)), + 4, new Common.AddressAndWeight[](0) ); // Try to set same config again - vm.expectRevert(abi.encodeWithSelector(Verifier.NonUniqueSignatures.selector)); - s_verifier.setConfigFromSource( - FEED_ID, - SOURCE_CHAIN_ID, - SOURCE_ADDRESS, - 1, + vm.expectRevert(abi.encodeWithSelector(Verifier.ConfigDigestAlreadySet.selector)); + s_verifier.setConfig( + configDigest, _getSignerAddresses(_getSigners(15)), - s_offchaintransmitters, 4, - bytes(""), - VERIFIER_VERSION, - bytes(""), new Common.AddressAndWeight[](0) ); } function test_incrementalConfigUpdates() public { // Set initial config - s_verifier.setConfigFromSource( + bytes32 configDigest1 = _configDigestFromConfigData( FEED_ID, - SOURCE_CHAIN_ID, + SOURCE_CHAIN_ID, SOURCE_ADDRESS, 1, _getSignerAddresses(_getSigners(15)), @@ -363,12 +485,18 @@ contract VerifierSetConfigWhenThereAreMultipleDigestsTest is BaseTestWithMultipl 4, bytes(""), VERIFIER_VERSION, - bytes(""), + bytes("") + ); + + s_verifier.setConfig( + configDigest1, + _getSignerAddresses(_getSigners(15)), + 4, new Common.AddressAndWeight[](0) ); - // Try to set same config again - s_verifier.setConfigFromSource( + // Set second config + bytes32 configDigest2 = _configDigestFromConfigData( FEED_ID, SOURCE_CHAIN_ID, SOURCE_ADDRESS, @@ -378,11 +506,18 @@ contract VerifierSetConfigWhenThereAreMultipleDigestsTest is BaseTestWithMultipl 4, bytes(""), VERIFIER_VERSION, - bytes(""), + bytes("") + ); + + s_verifier.setConfig( + configDigest2, + _getSignerAddresses(_getSigners(15)), + 4, new Common.AddressAndWeight[](0) ); - s_verifier.setConfigFromSource( + // Set third config + bytes32 configDigest3 = _configDigestFromConfigData( FEED_ID, SOURCE_CHAIN_ID, SOURCE_ADDRESS, @@ -392,7 +527,13 @@ contract VerifierSetConfigWhenThereAreMultipleDigestsTest is BaseTestWithMultipl 4, bytes(""), VERIFIER_VERSION, - bytes(""), + bytes("") + ); + + s_verifier.setConfig( + configDigest3, + _getSignerAddresses(_getSigners(15)), + 4, new Common.AddressAndWeight[](0) ); } diff --git a/contracts/src/v0.8/llo-feeds/v0.5.0/test/verifier/VerifierTest.t.sol b/contracts/src/v0.8/llo-feeds/v0.5.0/test/verifier/VerifierTest.t.sol index 7f8fc472c35..ec0268bb74a 100644 --- a/contracts/src/v0.8/llo-feeds/v0.5.0/test/verifier/VerifierTest.t.sol +++ b/contracts/src/v0.8/llo-feeds/v0.5.0/test/verifier/VerifierTest.t.sol @@ -14,8 +14,7 @@ contract VerifierConstructorTest is BaseTest { Verifier v = new Verifier(address(s_verifierProxy)); assertEq(v.owner(), ADMIN); - (uint32 configCount, uint32 blockNumber) = v.latestConfigDetails(FEED_ID); - assertEq(configCount, 0); + (uint32 blockNumber) = v.latestConfigDetails(FEED_ID); assertEq(blockNumber, 0); string memory typeAndVersion = s_verifier.typeAndVersion(); diff --git a/contracts/src/v0.8/llo-feeds/v0.5.0/test/verifier/VerifierVerifyTest.t.sol b/contracts/src/v0.8/llo-feeds/v0.5.0/test/verifier/VerifierVerifyTest.t.sol index 96306ede6ae..8441bf6d06b 100644 --- a/contracts/src/v0.8/llo-feeds/v0.5.0/test/verifier/VerifierVerifyTest.t.sol +++ b/contracts/src/v0.8/llo-feeds/v0.5.0/test/verifier/VerifierVerifyTest.t.sol @@ -234,7 +234,8 @@ contract VerifierVerifyMultipleConfigDigestTest is VerifierVerifyTest { function setUp() public override { VerifierVerifyTest.setUp(); s_oldConfigDigest = v1ConfigDigest; - s_verifier.setConfigFromSource( + + s_newConfigDigest = _configDigestFromConfigData( FEED_ID, SOURCE_CHAIN_ID, SOURCE_ADDRESS, @@ -244,20 +245,14 @@ contract VerifierVerifyMultipleConfigDigestTest is VerifierVerifyTest { FAULT_TOLERANCE_TWO, bytes(""), VERIFIER_VERSION, - bytes(""), - new Common.AddressAndWeight[](0) + bytes("") ); - s_newConfigDigest = _configDigestFromConfigData( - FEED_ID, - SOURCE_CHAIN_ID, - SOURCE_ADDRESS, - 2, + + s_verifier.setConfig( + s_newConfigDigest, _getSignerAddresses(_getSigners(20)), - s_offchaintransmitters, FAULT_TOLERANCE_TWO, - bytes(""), - VERIFIER_VERSION, - bytes("") + new Common.AddressAndWeight[](0) ); } @@ -311,4 +306,72 @@ contract VerifierVerifyMultipleConfigDigestTest is VerifierVerifyTest { changePrank(address(s_verifierProxy)); s_verifier.verify(signedReport, msg.sender); } + + function test_verifyAfterConfigUpdate() public { + // Get initial signers and set initial config + address[] memory initialSigners = _getSignerAddresses(_getSigners(15)); + bytes32 configDigest = keccak256("test"); + + s_verifier.setConfig( + configDigest, + initialSigners, + 4, + new Common.AddressAndWeight[](0) + ); + + // Get new signers and update config + address[] memory newSigners = _getSignerAddresses(_getSigners(20)); + s_verifier.updateConfig(configDigest, initialSigners, newSigners, 6); + + // Verify report with new signers should pass + s_reportContext[0] = configDigest; + bytes memory signedReportNewSigners = _generateV1EncodedBlob( + s_testReportOne, + s_reportContext, + _getSigners(7) // More than f=6 signers + ); + + bytes memory response = s_verifierProxy.verify(signedReportNewSigners, bytes("")); + assertReportsEqual(response, s_testReportOne); + + // Verify report with old signers should fail + bytes memory signedReportOldSigners = _generateV1EncodedBlob( + s_testReportOne, + s_reportContext, + _getSigners(5) // Old number of signers + ); + vm.expectRevert( + abi.encodeWithSelector(Verifier.IncorrectSignatureCount.selector, 5, 7) + ); + + s_verifierProxy.verify(signedReportOldSigners, bytes("")); + } + + function test_verifyAfterConfigUpdateWithExistingSigners() public { + // Get initial signers and set initial config + address[] memory signers = _getSignerAddresses(_getSigners(15)); + bytes32 configDigest = keccak256("test"); + + s_verifier.setConfig( + configDigest, + signers, + 4, + new Common.AddressAndWeight[](0) + ); + + // Update config with same signers and f + s_verifier.updateConfig(configDigest, signers, signers, 4); + + // Verify report should pass + s_reportContext[0] = configDigest; + bytes memory signedReport = _generateV1EncodedBlob( + s_testReportOne, + s_reportContext, + _getSigners(5) // More than f=4 signers + ); + + bytes memory response = s_verifierProxy.verify(signedReport, bytes("")); + assertReportsEqual(response, s_testReportOne); + + } }