From fae1da49b649fa30ef607373d4327880294fa384 Mon Sep 17 00:00:00 2001 From: Alex Sedighi Date: Tue, 23 Jul 2024 00:31:27 +0000 Subject: [PATCH 1/2] refactor(NODLMigration): prepare to create GrantsMigration --- script/CheckBridge.s.sol | 14 +-- src/bridge/BridgeBase.sol | 175 ++++++++++++++++++++++++++++++++ src/bridge/MigrationNFT.sol | 5 +- src/bridge/NODLMigration.sol | 99 ++---------------- test/bridge/NODLMigration.t.sol | 41 ++++---- 5 files changed, 217 insertions(+), 117 deletions(-) create mode 100644 src/bridge/BridgeBase.sol diff --git a/script/CheckBridge.s.sol b/script/CheckBridge.s.sol index e27760d2..558dcaec 100644 --- a/script/CheckBridge.s.sol +++ b/script/CheckBridge.s.sol @@ -5,6 +5,7 @@ pragma solidity 0.8.23; import {Script, console} from "forge-std/Script.sol"; import {NODLMigration} from "../src/bridge/NODLMigration.sol"; +import {BridgeBase} from "../src/bridge/BridgeBase.sol"; contract CheckBridge is Script { NODLMigration internal bridge; @@ -16,20 +17,19 @@ contract CheckBridge is Script { } function run() public view { - (address target, uint256 amount, uint256 lastVote, uint8 totalVotes, bool executed) = - bridge.proposals(proposalId); + (address target, uint256 amount, BridgeBase.ProposalStatus memory status) = bridge.proposals(proposalId); console.log("Proposal targets %s with %d NODL", address(target), amount); - console.log("Proposal has %d votes", totalVotes); + console.log("Proposal has %d votes", status.totalVotes); - if (executed) { + if (status.executed) { console.log("Proposal has already been executed"); } else { console.log("Proposal has not been executed"); - if (totalVotes >= bridge.threshold()) { - uint256 blocksPassed = block.number - lastVote; - if (block.number - lastVote >= bridge.delay()) { + if (status.totalVotes >= bridge.threshold()) { + uint256 blocksPassed = block.number - status.lastVote; + if (block.number - status.lastVote >= bridge.delay()) { console.log("Proposal has enough votes to execute and should be executed soon"); } else { console.log("Proposal has enough votes but needs to wait %d blocks", bridge.delay() - blocksPassed); diff --git a/src/bridge/BridgeBase.sol b/src/bridge/BridgeBase.sol new file mode 100644 index 00000000..7ff65981 --- /dev/null +++ b/src/bridge/BridgeBase.sol @@ -0,0 +1,175 @@ +// SPDX-License-Identifier: BSD-3-Clause-Clear + +pragma solidity 0.8.23; + +import {NODL} from "../NODL.sol"; + +/// @title BridgeBase +/// @notice Abstract contract for bridging free or vested tokens with the help of oracles. +/// @dev This contract provides basic functionalities for voting on proposals +/// to bridge tokens and ensuring certain constraints such as voting thresholds and delays. +abstract contract BridgeBase { + /// @notice Struct to store the status of a proposal. + /// @dev Tracks the last block number when a vote was cast, the total number of votes, + /// and whether the proposal has been executed. + struct ProposalStatus { + uint256 lastVote; + uint8 totalVotes; + bool executed; + } + + /// @notice Token contract address for the NODL token. + NODL public nodl; + + /// @notice Mapping to track whether an address is an oracle. + mapping(address => bool) public isOracle; + + /// @notice Minimum number of votes needed to execute a proposal. + uint8 public threshold; + + /// @notice Number of blocks to delay before a proposal can be executed after reaching the voting threshold. + uint256 public delay; + + /// @notice Maximum number of oracles allowed. + uint8 public constant MAX_ORACLES = 10; + + /// @notice Mapping of oracles to proposals to track which oracle has voted on which proposal. + mapping(address => mapping(bytes32 => bool)) public voted; + + /// @notice Emitted when the first vote a proposal has been cast. + event VoteStarted(bytes32 indexed proposal, address oracle, address indexed user, uint256 amount); + + /// @notice Emitted when an oracle votes on a proposal which is already created. + event Voted(bytes32 indexed proposal, address oracle); + + /// @notice Error to indicate an oracle has already voted on a proposal. + error AlreadyVoted(bytes32 proposal, address oracle); + + /// @notice Error to indicate a proposal has already been executed. + error AlreadyExecuted(bytes32 proposal); + + /// @notice Error to indicate parameters of a proposal have been changed after initiation. + error ParametersChanged(bytes32 proposal); + + /// @notice Error to indicate an address is not recognized as an oracle. + error NotAnOracle(address user); + + /// @notice Error to indicate it's too soon to execute a proposal. + /// Please note `withdraw` here refers to a function for free tokens where they are minted for the user. + /// We have kept the name for API compatibility. + error NotYetWithdrawable(bytes32 proposal); + + /// @notice Error to indicate insufficient votes to execute a proposal. + error NotEnoughVotes(bytes32 proposal); + + /// @notice Error to indicate that the number of oracles exceeds the allowed maximum. + error MaxOraclesExceeded(); + + /// @notice Initializes the contract with specified parameters. + /// @param bridgeOracles Array of oracle accounts. + /// @param token Contract address of the NODL token. + /// @param minVotes Minimum required votes to consider a proposal valid. + /// @param minDelay Blocks to wait before a passed proposal can be executed. + constructor(address[] memory bridgeOracles, NODL token, uint8 minVotes, uint256 minDelay) { + require(bridgeOracles.length >= minVotes, "Not enough oracles"); + require(minVotes > 0, "Votes must be more than zero"); + + _mustNotExceedMaxOracles(bridgeOracles.length); + + for (uint256 i = 0; i < bridgeOracles.length; i++) { + isOracle[bridgeOracles[i]] = true; + } + + nodl = token; + threshold = minVotes; + delay = minDelay; + } + + /// @notice Returns the current status of a proposal. + /// @dev Must be implemented by inheriting contracts. + /// @param proposal The hash identifier of the proposal. + /// @return The storage pointer to the status of the proposal. + function _proposalStatus(bytes32 proposal) internal view virtual returns (ProposalStatus storage); + + /// @notice Checks if a proposal already exists. + /// @param proposal The hash identifier of the proposal. + /// @return True if the proposal exists and has votes. + function _proposalExists(bytes32 proposal) internal view returns (bool) { + ProposalStatus storage status = _proposalStatus(proposal); + return status.totalVotes > 0; + } + + /// @notice Creates a new vote on a proposal. + /// @dev Sets initial values and emits a VoteStarted event. + /// @param proposal The hash identifier of the proposal. + /// @param oracle The oracle address initiating the vote. + /// @param user The user address associated with the vote. + /// @param amount The amount of tokens being bridged. + function _createVote(bytes32 proposal, address oracle, address user, uint256 amount) internal virtual { + _mustNotHaveVotedYet(proposal, oracle); + voted[oracle][proposal] = true; + ProposalStatus storage status = _proposalStatus(proposal); + status.totalVotes = 1; + status.lastVote = block.number; + emit VoteStarted(proposal, oracle, user, amount); + } + + /// @notice Records a vote for a proposal by an oracle. + /// @param proposal The hash identifier of the proposal. + /// @param oracle The oracle casting the vote. + function _recordVote(bytes32 proposal, address oracle) internal virtual { + _mustNotHaveVotedYet(proposal, oracle); + voted[oracle][proposal] = true; + ProposalStatus storage status = _proposalStatus(proposal); + status.totalVotes += 1; + status.lastVote = block.number; + emit Voted(proposal, oracle); + } + + /// @notice Executes a proposal after all conditions are met. + /// @param proposal The hash identifier of the proposal. + function _execute(bytes32 proposal) internal { + _mustNotHaveExecutedYet(proposal); + _mustHaveEnoughVotes(proposal); + _mustBePastSafetyDelay(proposal); + + ProposalStatus storage status = _proposalStatus(proposal); + status.executed = true; + } + + function _mustNotHaveExecutedYet(bytes32 proposal) internal view { + if (_proposalStatus(proposal).executed) { + revert AlreadyExecuted(proposal); + } + } + + function _mustBePastSafetyDelay(bytes32 proposal) internal view { + if (block.number - _proposalStatus(proposal).lastVote < delay) { + revert NotYetWithdrawable(proposal); + } + } + + function _mustHaveEnoughVotes(bytes32 proposal) internal view { + if (_proposalStatus(proposal).totalVotes < threshold) { + revert NotEnoughVotes(proposal); + } + } + + function _mustBeAnOracle(address maybeOracle) internal view { + if (!isOracle[maybeOracle]) { + revert NotAnOracle(maybeOracle); + } + } + + function _mustNotExceedMaxOracles(uint256 length) internal pure { + if (length > MAX_ORACLES) { + revert MaxOraclesExceeded(); + } + } + + function _mustNotHaveVotedYet(bytes32 proposal, address oracle) internal view { + if (voted[oracle][proposal]) { + revert AlreadyVoted(proposal, oracle); + } + } +} diff --git a/src/bridge/MigrationNFT.sol b/src/bridge/MigrationNFT.sol index 0140c5ab..58662c15 100644 --- a/src/bridge/MigrationNFT.sol +++ b/src/bridge/MigrationNFT.sol @@ -5,6 +5,7 @@ pragma solidity 0.8.23; import {ERC721} from "openzeppelin-contracts/contracts/token/ERC721/ERC721.sol"; import {Strings} from "openzeppelin-contracts/contracts/utils/Strings.sol"; import {NODLMigration} from "./NODLMigration.sol"; +import {BridgeBase} from "./BridgeBase.sol"; contract MigrationNFT is ERC721 { uint256 public nextTokenId; @@ -83,10 +84,10 @@ contract MigrationNFT is ERC721 { function safeMint(bytes32 txHash) public { _mustNotHaveBeenClaimed(txHash); - (address target, uint256 amount,,, bool executed) = migration.proposals(txHash); + (address target, uint256 amount, BridgeBase.ProposalStatus memory status) = migration.proposals(txHash); _mustBeAnExistingProposal(target); - _mustBeExecuted(executed); + _mustBeExecuted(status.executed); bool alreadyHolder = _mustAlreadyBeHolderOrEnougHoldersRemaining(target); (uint256[] memory levelsToMint, uint256 nbLevelsToMint) = _computeLevelUps(target, amount); diff --git a/src/bridge/NODLMigration.sol b/src/bridge/NODLMigration.sol index ee31dfcb..1789aa13 100644 --- a/src/bridge/NODLMigration.sol +++ b/src/bridge/NODLMigration.sol @@ -3,56 +3,27 @@ pragma solidity 0.8.23; import {NODL} from "../NODL.sol"; +import {BridgeBase} from "./BridgeBase.sol"; /// @title NODLMigration /// @notice This contract is used to help migrating the NODL assets from the Nodle Parachain /// to the ZkSync contracts. -contract NODLMigration { +contract NODLMigration is BridgeBase { struct Proposal { address target; uint256 amount; - uint256 lastVote; - uint8 totalVotes; - bool executed; + ProposalStatus status; } - NODL public nodl; - mapping(address => bool) public isOracle; - uint8 public threshold; - uint256 public delay; - // We track votes in a seperate mapping to avoid having to write helper functions to // expose the votes for each proposal. mapping(bytes32 => Proposal) public proposals; - mapping(address => mapping(bytes32 => bool)) public voted; - - error AlreadyVoted(bytes32 proposal, address oracle); - error AlreadyExecuted(bytes32 proposal); - error ParametersChanged(bytes32 proposal); - error NotAnOracle(address user); - error NotYetWithdrawable(bytes32 proposal); - error NotEnoughVotes(bytes32 proposal); - event VoteStarted(bytes32 indexed proposal, address oracle, address indexed user, uint256 amount); - event Voted(bytes32 indexed proposal, address oracle); event Withdrawn(bytes32 indexed proposal, address indexed user, uint256 amount); - /// @param bridgeOracles Array of oracle accounts that will be able to bridge the tokens. - /// @param token Contract address of the NODL token. - /// @param minVotes Minimum number of votes required to bridge the tokens. This needs to be - /// less than or equal to the number of oracles and is expected to be above 1. - /// @param minDelay Minimum delay in blocks before bridged tokens can be minted. - constructor(address[] memory bridgeOracles, NODL token, uint8 minVotes, uint256 minDelay) { - assert(bridgeOracles.length >= minVotes); - assert(minVotes > 0); - - for (uint256 i = 0; i < bridgeOracles.length; i++) { - isOracle[bridgeOracles[i]] = true; - } - nodl = token; - threshold = minVotes; - delay = minDelay; - } + constructor(address[] memory bridgeOracles, NODL token, uint8 minVotes, uint256 minDelay) + BridgeBase(bridgeOracles, token, minVotes, minDelay) + {} /// @notice Bridge some tokens from the Nodle Parachain to the ZkSync contracts. This /// tracks "votes" from each oracle and unlocks execution after a withdrawal delay. @@ -76,76 +47,28 @@ contract NODLMigration { /// proposal has enough votes and has passed the safety delay. /// @param paraTxHash The transaction hash on the Parachain for this transfer. function withdraw(bytes32 paraTxHash) external { - _mustNotHaveExecutedYet(paraTxHash); - _mustHaveEnoughVotes(paraTxHash); - _mustBePastSafetyDelay(paraTxHash); - + _execute(paraTxHash); _withdraw(paraTxHash, proposals[paraTxHash].target, proposals[paraTxHash].amount); } - function _mustBeAnOracle(address maybeOracle) internal view { - if (!isOracle[maybeOracle]) { - revert NotAnOracle(maybeOracle); - } - } - - function _mustNotHaveVotedYet(bytes32 proposal, address oracle) internal view { - if (voted[oracle][proposal]) { - revert AlreadyVoted(proposal, oracle); - } - } - - function _mustNotHaveExecutedYet(bytes32 proposal) internal view { - if (proposals[proposal].executed) { - revert AlreadyExecuted(proposal); - } - } - function _mustNotBeChangingParameters(bytes32 proposal, address user, uint256 amount) internal view { if (proposals[proposal].amount != amount || proposals[proposal].target != user) { revert ParametersChanged(proposal); } } - function _mustBePastSafetyDelay(bytes32 proposal) internal view { - if (block.number - proposals[proposal].lastVote < delay) { - revert NotYetWithdrawable(proposal); - } + function _proposalStatus(bytes32 proposal) internal view override returns (ProposalStatus storage) { + return proposals[proposal].status; } - function _mustHaveEnoughVotes(bytes32 proposal) internal view { - if (proposals[proposal].totalVotes < threshold) { - revert NotEnoughVotes(proposal); - } - } - - function _proposalExists(bytes32 proposal) internal view returns (bool) { - return proposals[proposal].totalVotes > 0 && proposals[proposal].amount > 0; - } - - function _createVote(bytes32 proposal, address oracle, address user, uint256 amount) internal { - voted[oracle][proposal] = true; + function _createVote(bytes32 proposal, address oracle, address user, uint256 amount) internal override { proposals[proposal].target = user; proposals[proposal].amount = amount; - proposals[proposal].totalVotes = 1; - proposals[proposal].lastVote = block.number; - - emit VoteStarted(proposal, oracle, user, amount); - } - - function _recordVote(bytes32 proposal, address oracle) internal { - voted[oracle][proposal] = true; - // this is safe since we are unlikely to have maxUint8 oracles to manage - proposals[proposal].totalVotes += 1; - proposals[proposal].lastVote = block.number; - - emit Voted(proposal, oracle); + super._createVote(proposal, oracle, user, amount); } function _withdraw(bytes32 proposal, address user, uint256 amount) internal { - proposals[proposal].executed = true; nodl.mint(user, amount); - emit Withdrawn(proposal, user, amount); } } diff --git a/test/bridge/NODLMigration.t.sol b/test/bridge/NODLMigration.t.sol index f89f0005..a6629f9b 100644 --- a/test/bridge/NODLMigration.t.sol +++ b/test/bridge/NODLMigration.t.sol @@ -4,6 +4,7 @@ pragma solidity ^0.8.20; import {Test, console} from "forge-std/Test.sol"; import {NODLMigration} from "../../src/bridge/NODLMigration.sol"; +import {BridgeBase} from "../../src/bridge/BridgeBase.sol"; import {NODL} from "../../src/NODL.sol"; contract NODLMigrationTest is Test { @@ -34,7 +35,7 @@ contract NODLMigrationTest is Test { } function test_nonOracleMayNotBridgeTokens() public { - vm.expectRevert(abi.encodeWithSelector(NODLMigration.NotAnOracle.selector, user)); + vm.expectRevert(abi.encodeWithSelector(BridgeBase.NotAnOracle.selector, user)); vm.prank(user); migration.bridge(0x0, user, 100); } @@ -44,7 +45,7 @@ contract NODLMigrationTest is Test { migration.bridge(0x0, user, 100); - vm.expectRevert(abi.encodeWithSelector(NODLMigration.AlreadyVoted.selector, 0x0, oracles[0])); + vm.expectRevert(abi.encodeWithSelector(BridgeBase.AlreadyVoted.selector, 0x0, oracles[0])); migration.bridge(0x0, user, 100); vm.stopPrank(); @@ -60,7 +61,7 @@ contract NODLMigrationTest is Test { vm.roll(block.number + delay + 1); migration.withdraw(0x0); - vm.expectRevert(abi.encodeWithSelector(NODLMigration.AlreadyExecuted.selector, 0x0)); + vm.expectRevert(abi.encodeWithSelector(BridgeBase.AlreadyExecuted.selector, 0x0)); vm.prank(oracles[2]); migration.bridge(0x0, user, 100); } @@ -69,46 +70,46 @@ contract NODLMigrationTest is Test { vm.prank(oracles[0]); migration.bridge(0x0, user, 100); - vm.expectRevert(abi.encodeWithSelector(NODLMigration.ParametersChanged.selector, 0x0)); + vm.expectRevert(abi.encodeWithSelector(BridgeBase.ParametersChanged.selector, 0x0)); vm.prank(oracles[1]); migration.bridge(0x0, user, 200); - vm.expectRevert(abi.encodeWithSelector(NODLMigration.ParametersChanged.selector, 0x0)); + vm.expectRevert(abi.encodeWithSelector(BridgeBase.ParametersChanged.selector, 0x0)); vm.prank(oracles[1]); migration.bridge(0x0, oracles[1], 100); } function test_recordsVotes() public { vm.expectEmit(); - emit NODLMigration.VoteStarted(0x0, oracles[0], user, 100); + emit BridgeBase.VoteStarted(0x0, oracles[0], user, 100); vm.prank(oracles[0]); migration.bridge(0x0, user, 100); - (address target, uint256 amount, uint256 lastVote, uint256 totalVotes, bool executed) = migration.proposals(0x0); + (address target, uint256 amount, BridgeBase.ProposalStatus memory status) = migration.proposals(0x0); assertEq(target, user); assertEq(amount, 100); - assertEq(lastVote, block.number); - assertEq(totalVotes, 1); - assertEq(executed, false); + assertEq(status.lastVote, block.number); + assertEq(status.totalVotes, 1); + assertEq(status.executed, false); vm.expectEmit(); - emit NODLMigration.Voted(0x0, oracles[1]); + emit BridgeBase.Voted(0x0, oracles[1]); vm.prank(oracles[1]); migration.bridge(0x0, user, 100); - (target, amount, lastVote, totalVotes, executed) = migration.proposals(0x0); + (target, amount, status) = migration.proposals(0x0); assertEq(target, user); assertEq(amount, 100); - assertEq(lastVote, block.number); - assertEq(totalVotes, 2); - assertEq(executed, false); + assertEq(status.lastVote, block.number); + assertEq(status.totalVotes, 2); + assertEq(status.executed, false); } function test_mayNotWithdrawIfNotEnoughVotes() public { vm.prank(oracles[0]); migration.bridge(0x0, user, 100); - vm.expectRevert(abi.encodeWithSelector(NODLMigration.NotEnoughVotes.selector, 0x0)); + vm.expectRevert(abi.encodeWithSelector(BridgeBase.NotEnoughVotes.selector, 0x0)); migration.withdraw(0x0); } @@ -119,7 +120,7 @@ contract NODLMigrationTest is Test { vm.prank(oracles[1]); migration.bridge(0x0, user, 100); - vm.expectRevert(abi.encodeWithSelector(NODLMigration.NotYetWithdrawable.selector, 0x0)); + vm.expectRevert(abi.encodeWithSelector(BridgeBase.NotYetWithdrawable.selector, 0x0)); migration.withdraw(0x0); } @@ -134,7 +135,7 @@ contract NODLMigrationTest is Test { migration.withdraw(0x0); - vm.expectRevert(abi.encodeWithSelector(NODLMigration.AlreadyExecuted.selector, 0x0)); + vm.expectRevert(abi.encodeWithSelector(BridgeBase.AlreadyExecuted.selector, 0x0)); migration.withdraw(0x0); } @@ -152,8 +153,8 @@ contract NODLMigrationTest is Test { vm.prank(user); // anybody can call withdraw migration.withdraw(0x0); - (,,,, bool executed) = migration.proposals(0x0); - assertEq(executed, true); + (,, BridgeBase.ProposalStatus memory status) = migration.proposals(0x0); + assertEq(status.executed, true); assertEq(nodl.balanceOf(user), 100); } From 59038aef42a6001311ebc29fe57c02dcc2b4aae7 Mon Sep 17 00:00:00 2001 From: Alex Sedighi Date: Tue, 23 Jul 2024 23:12:00 +0000 Subject: [PATCH 2/2] feat(NODLMigration): keep it backward compatible, avoid changing the storage layout --- script/CheckBridge.s.sol | 14 +++--- src/bridge/BridgeBase.sol | 84 ++++++++++++++++++++------------- src/bridge/MigrationNFT.sol | 5 +- src/bridge/NODLMigration.sol | 32 +++++++++++-- test/bridge/NODLMigration.t.sol | 20 ++++---- 5 files changed, 98 insertions(+), 57 deletions(-) diff --git a/script/CheckBridge.s.sol b/script/CheckBridge.s.sol index 558dcaec..e27760d2 100644 --- a/script/CheckBridge.s.sol +++ b/script/CheckBridge.s.sol @@ -5,7 +5,6 @@ pragma solidity 0.8.23; import {Script, console} from "forge-std/Script.sol"; import {NODLMigration} from "../src/bridge/NODLMigration.sol"; -import {BridgeBase} from "../src/bridge/BridgeBase.sol"; contract CheckBridge is Script { NODLMigration internal bridge; @@ -17,19 +16,20 @@ contract CheckBridge is Script { } function run() public view { - (address target, uint256 amount, BridgeBase.ProposalStatus memory status) = bridge.proposals(proposalId); + (address target, uint256 amount, uint256 lastVote, uint8 totalVotes, bool executed) = + bridge.proposals(proposalId); console.log("Proposal targets %s with %d NODL", address(target), amount); - console.log("Proposal has %d votes", status.totalVotes); + console.log("Proposal has %d votes", totalVotes); - if (status.executed) { + if (executed) { console.log("Proposal has already been executed"); } else { console.log("Proposal has not been executed"); - if (status.totalVotes >= bridge.threshold()) { - uint256 blocksPassed = block.number - status.lastVote; - if (block.number - status.lastVote >= bridge.delay()) { + if (totalVotes >= bridge.threshold()) { + uint256 blocksPassed = block.number - lastVote; + if (block.number - lastVote >= bridge.delay()) { console.log("Proposal has enough votes to execute and should be executed soon"); } else { console.log("Proposal has enough votes but needs to wait %d blocks", bridge.delay() - blocksPassed); diff --git a/src/bridge/BridgeBase.sol b/src/bridge/BridgeBase.sol index 7ff65981..0bfbd241 100644 --- a/src/bridge/BridgeBase.sol +++ b/src/bridge/BridgeBase.sol @@ -9,15 +9,6 @@ import {NODL} from "../NODL.sol"; /// @dev This contract provides basic functionalities for voting on proposals /// to bridge tokens and ensuring certain constraints such as voting thresholds and delays. abstract contract BridgeBase { - /// @notice Struct to store the status of a proposal. - /// @dev Tracks the last block number when a vote was cast, the total number of votes, - /// and whether the proposal has been executed. - struct ProposalStatus { - uint256 lastVote; - uint8 totalVotes; - bool executed; - } - /// @notice Token contract address for the NODL token. NODL public nodl; @@ -85,20 +76,6 @@ abstract contract BridgeBase { delay = minDelay; } - /// @notice Returns the current status of a proposal. - /// @dev Must be implemented by inheriting contracts. - /// @param proposal The hash identifier of the proposal. - /// @return The storage pointer to the status of the proposal. - function _proposalStatus(bytes32 proposal) internal view virtual returns (ProposalStatus storage); - - /// @notice Checks if a proposal already exists. - /// @param proposal The hash identifier of the proposal. - /// @return True if the proposal exists and has votes. - function _proposalExists(bytes32 proposal) internal view returns (bool) { - ProposalStatus storage status = _proposalStatus(proposal); - return status.totalVotes > 0; - } - /// @notice Creates a new vote on a proposal. /// @dev Sets initial values and emits a VoteStarted event. /// @param proposal The hash identifier of the proposal. @@ -107,10 +84,10 @@ abstract contract BridgeBase { /// @param amount The amount of tokens being bridged. function _createVote(bytes32 proposal, address oracle, address user, uint256 amount) internal virtual { _mustNotHaveVotedYet(proposal, oracle); + voted[oracle][proposal] = true; - ProposalStatus storage status = _proposalStatus(proposal); - status.totalVotes = 1; - status.lastVote = block.number; + _incTotalVotes(proposal); + _updateLastVote(proposal, block.number); emit VoteStarted(proposal, oracle, user, amount); } @@ -119,10 +96,10 @@ abstract contract BridgeBase { /// @param oracle The oracle casting the vote. function _recordVote(bytes32 proposal, address oracle) internal virtual { _mustNotHaveVotedYet(proposal, oracle); + voted[oracle][proposal] = true; - ProposalStatus storage status = _proposalStatus(proposal); - status.totalVotes += 1; - status.lastVote = block.number; + _incTotalVotes(proposal); + _updateLastVote(proposal, block.number); emit Voted(proposal, oracle); } @@ -133,24 +110,63 @@ abstract contract BridgeBase { _mustHaveEnoughVotes(proposal); _mustBePastSafetyDelay(proposal); - ProposalStatus storage status = _proposalStatus(proposal); - status.executed = true; + _flagAsExecuted(proposal); } + /** + * @dev Updates the last vote value for a given proposal. + * @param proposal The identifier of the proposal. + * @param value The new value for the last vote. + */ + function _updateLastVote(bytes32 proposal, uint256 value) internal virtual; + + /** + * @dev Increments the total votes count for a given proposal. + * @param proposal The identifier of the proposal. + */ + function _incTotalVotes(bytes32 proposal) internal virtual; + + /** + * @dev Flags a proposal as executed. + * @param proposal The identifier of the proposal. + */ + function _flagAsExecuted(bytes32 proposal) internal virtual; + + /** + * @dev Retrieves the last vote value for a given proposal. + * @param proposal The identifier of the proposal. + * @return The last vote value. + */ + function _lastVote(bytes32 proposal) internal view virtual returns (uint256); + + /** + * @dev Retrieves the total votes count for a given proposal. + * @param proposal The identifier of the proposal. + * @return The total votes count. + */ + function _totalVotes(bytes32 proposal) internal view virtual returns (uint8); + + /** + * @dev Checks if a proposal has been executed. + * @param proposal The identifier of the proposal. + * @return A boolean indicating if the proposal has been executed. + */ + function _executed(bytes32 proposal) internal view virtual returns (bool); + function _mustNotHaveExecutedYet(bytes32 proposal) internal view { - if (_proposalStatus(proposal).executed) { + if (_executed(proposal)) { revert AlreadyExecuted(proposal); } } function _mustBePastSafetyDelay(bytes32 proposal) internal view { - if (block.number - _proposalStatus(proposal).lastVote < delay) { + if (block.number - _lastVote(proposal) < delay) { revert NotYetWithdrawable(proposal); } } function _mustHaveEnoughVotes(bytes32 proposal) internal view { - if (_proposalStatus(proposal).totalVotes < threshold) { + if (_totalVotes(proposal) < threshold) { revert NotEnoughVotes(proposal); } } diff --git a/src/bridge/MigrationNFT.sol b/src/bridge/MigrationNFT.sol index 58662c15..0140c5ab 100644 --- a/src/bridge/MigrationNFT.sol +++ b/src/bridge/MigrationNFT.sol @@ -5,7 +5,6 @@ pragma solidity 0.8.23; import {ERC721} from "openzeppelin-contracts/contracts/token/ERC721/ERC721.sol"; import {Strings} from "openzeppelin-contracts/contracts/utils/Strings.sol"; import {NODLMigration} from "./NODLMigration.sol"; -import {BridgeBase} from "./BridgeBase.sol"; contract MigrationNFT is ERC721 { uint256 public nextTokenId; @@ -84,10 +83,10 @@ contract MigrationNFT is ERC721 { function safeMint(bytes32 txHash) public { _mustNotHaveBeenClaimed(txHash); - (address target, uint256 amount, BridgeBase.ProposalStatus memory status) = migration.proposals(txHash); + (address target, uint256 amount,,, bool executed) = migration.proposals(txHash); _mustBeAnExistingProposal(target); - _mustBeExecuted(status.executed); + _mustBeExecuted(executed); bool alreadyHolder = _mustAlreadyBeHolderOrEnougHoldersRemaining(target); (uint256[] memory levelsToMint, uint256 nbLevelsToMint) = _computeLevelUps(target, amount); diff --git a/src/bridge/NODLMigration.sol b/src/bridge/NODLMigration.sol index 1789aa13..9b72ad36 100644 --- a/src/bridge/NODLMigration.sol +++ b/src/bridge/NODLMigration.sol @@ -12,7 +12,9 @@ contract NODLMigration is BridgeBase { struct Proposal { address target; uint256 amount; - ProposalStatus status; + uint256 lastVote; + uint8 totalVotes; + bool executed; } // We track votes in a seperate mapping to avoid having to write helper functions to @@ -57,8 +59,8 @@ contract NODLMigration is BridgeBase { } } - function _proposalStatus(bytes32 proposal) internal view override returns (ProposalStatus storage) { - return proposals[proposal].status; + function _proposalExists(bytes32 proposal) internal view returns (bool) { + return proposals[proposal].totalVotes > 0 && proposals[proposal].amount > 0; } function _createVote(bytes32 proposal, address oracle, address user, uint256 amount) internal override { @@ -71,4 +73,28 @@ contract NODLMigration is BridgeBase { nodl.mint(user, amount); emit Withdrawn(proposal, user, amount); } + + function _flagAsExecuted(bytes32 proposal) internal override { + proposals[proposal].executed = true; + } + + function _incTotalVotes(bytes32 proposal) internal override { + proposals[proposal].totalVotes++; + } + + function _updateLastVote(bytes32 proposal, uint256 value) internal override { + proposals[proposal].lastVote = value; + } + + function _totalVotes(bytes32 proposal) internal view override returns (uint8) { + return proposals[proposal].totalVotes; + } + + function _lastVote(bytes32 proposal) internal view override returns (uint256) { + return proposals[proposal].lastVote; + } + + function _executed(bytes32 proposal) internal view override returns (bool) { + return proposals[proposal].executed; + } } diff --git a/test/bridge/NODLMigration.t.sol b/test/bridge/NODLMigration.t.sol index a6629f9b..3baf4ad2 100644 --- a/test/bridge/NODLMigration.t.sol +++ b/test/bridge/NODLMigration.t.sol @@ -85,24 +85,24 @@ contract NODLMigrationTest is Test { vm.prank(oracles[0]); migration.bridge(0x0, user, 100); - (address target, uint256 amount, BridgeBase.ProposalStatus memory status) = migration.proposals(0x0); + (address target, uint256 amount, uint256 lastVote, uint256 totalVotes, bool executed) = migration.proposals(0x0); assertEq(target, user); assertEq(amount, 100); - assertEq(status.lastVote, block.number); - assertEq(status.totalVotes, 1); - assertEq(status.executed, false); + assertEq(lastVote, block.number); + assertEq(totalVotes, 1); + assertEq(executed, false); vm.expectEmit(); emit BridgeBase.Voted(0x0, oracles[1]); vm.prank(oracles[1]); migration.bridge(0x0, user, 100); - (target, amount, status) = migration.proposals(0x0); + (target, amount, lastVote, totalVotes, executed) = migration.proposals(0x0); assertEq(target, user); assertEq(amount, 100); - assertEq(status.lastVote, block.number); - assertEq(status.totalVotes, 2); - assertEq(status.executed, false); + assertEq(lastVote, block.number); + assertEq(totalVotes, 2); + assertEq(executed, false); } function test_mayNotWithdrawIfNotEnoughVotes() public { @@ -153,8 +153,8 @@ contract NODLMigrationTest is Test { vm.prank(user); // anybody can call withdraw migration.withdraw(0x0); - (,, BridgeBase.ProposalStatus memory status) = migration.proposals(0x0); - assertEq(status.executed, true); + (,,,, bool executed) = migration.proposals(0x0); + assertEq(executed, true); assertEq(nodl.balanceOf(user), 100); }