diff --git a/contracts/ServiceNodeRewards.sol b/contracts/ServiceNodeRewards.sol index 896dbad..89cfb57 100644 --- a/contracts/ServiceNodeRewards.sol +++ b/contracts/ServiceNodeRewards.sol @@ -29,12 +29,12 @@ contract ServiceNodeRewards is Initializable, Ownable2StepUpgradeable, PausableU // contributor may not initiate a leave request within the initial LEAVE_DELAY: uint256 public constant SMALL_CONTRIBUTOR_LEAVE_DELAY = 30 days; uint256 public constant SMALL_CONTRIBUTOR_DIVISOR = 4; - // Minimum time before a node may be liquidated. This prevents front-running liquidations where - // a malicious entity could obtain a "not on the network" signature from service nodes in the - // short period before the registration is observed on the Oxen chain. 2 hours matches the - // initial decommission credit of Oxen nodes (and so shortly over 2 hours is the soonest we - // could expect to see a legitimate deregistration/liquidation request). - uint256 public constant MINIMUM_LIQUIDATION_AGE = 2 hours; + // Minimum time before a node may exit (normally or via liquidation). This prevents + // front-running exits where a malicious entity could obtain a "not on the network" signature + // from service nodes in the short period before the registration is observed on the Oxen chain. + // 2 hours matches the initial decommission credit of Oxen nodes (and so shortly over 2 hours is + // the soonest we could expect to see a legitimate deregistration/liquidation request). + uint256 public constant MINIMUM_EXIT_AGE = 2 hours; uint64 public nextServiceNodeID; uint256 public totalNodes; @@ -211,7 +211,7 @@ contract ServiceNodeRewards is Initializable, Ownable2StepUpgradeable, PausableU error InsufficientNodes(); error InvalidBLSSignature(BN256G1.G1Point aggPubkey); error InvalidBLSProofOfPossession(); - error LiquidationTooEarly(uint64 serviceNodeID, uint256 addedTimestamp, uint256 currenttime); + error ExitTooEarly(uint64 serviceNodeID, uint256 addedTimestamp, uint256 currenttime); /// @param endTimestamp Timestamp that must be met to permit a leave request /// @param currTimestamp Timestamp of the current block @@ -542,24 +542,12 @@ contract ServiceNodeRewards is Initializable, Ownable2StepUpgradeable, PausableU BLSSignatureParams calldata blsSignature, uint64[] memory ids ) external whenNotPaused whenStarted hasEnoughSigners(ids.length) { - bytes memory pubkeyBytes = BN256G1.getKeyForG1Point(blsPubkey); - uint64 serviceNodeID = serviceNodeIDs[pubkeyBytes]; - if (serviceNodeID == 0) - revert BLSPubkeyDoesNotExist(blsPubkey); - if (signatureTimestampHasExpired(timestamp)) { - revert SignatureExpired(serviceNodeID, timestamp, block.timestamp); - } - if ( - blsPubkey.X != _serviceNodes[serviceNodeID].blsPubkey.X || blsPubkey.Y != _serviceNodes[serviceNodeID].blsPubkey.Y - ) revert BLSPubkeyDoesNotMatch(serviceNodeID, blsPubkey); + (uint64 serviceNodeID, ServiceNode memory node) = _validateBLSExitWithSignature( + blsPubkey, timestamp, blsSignature, exitTag, ids); - // NOTE: Validate signature - { - bytes memory encodedMessage = abi.encodePacked(exitTag, blsPubkey.X, blsPubkey.Y, timestamp); - BN256G2.G2Point memory Hm = BN256G2.hashToG2(encodedMessage, hashToG2Tag); - validateSignatureOrRevert(ids, blsSignature, Hm); - } + if (node.leaveRequestTimestamp == 0) + revert LeaveRequestNotInitiatedYet(serviceNodeID); _exitBLSPublicKey(serviceNodeID, _serviceNodes[serviceNodeID].deposit); } @@ -597,6 +585,43 @@ contract ServiceNodeRewards is Initializable, Ownable2StepUpgradeable, PausableU emit ServiceNodeExit(serviceNodeID, operator, returnedAmount, pubkey); } + /// @dev Internal function to handle common liquidate/exit checks when + /// exiting/liquidating with a service node network signature. + /// + /// @return serviceNodeID the ID of the service node containing `blsPubkey` + /// @return node the ServiceNode info for the node + function _validateBLSExitWithSignature( + BN256G1.G1Point calldata blsPubkey, + uint256 timestamp, + BLSSignatureParams calldata blsSignature, + bytes32 signatureTag, + uint64[] memory ids + ) internal returns (uint64 serviceNodeID, ServiceNode memory node) { + + bytes memory pubkeyBytes = BN256G1.getKeyForG1Point(blsPubkey); + serviceNodeID = serviceNodeIDs[pubkeyBytes]; + if (serviceNodeID == 0) + revert BLSPubkeyDoesNotExist(blsPubkey); + if (signatureTimestampHasExpired(timestamp)) { + revert SignatureExpired(serviceNodeID, timestamp, block.timestamp); + } + + node = _serviceNodes[serviceNodeID]; + if (blsPubkey.X != node.blsPubkey.X || blsPubkey.Y != node.blsPubkey.Y) { + revert BLSPubkeyDoesNotMatch(serviceNodeID, blsPubkey); + } + + if (block.timestamp < node.addedTimestamp + MINIMUM_EXIT_AGE) + revert ExitTooEarly(serviceNodeID, node.addedTimestamp, block.timestamp); + + // NOTE: Validate signature + { + bytes memory encodedMessage = abi.encodePacked(signatureTag, blsPubkey.X, blsPubkey.Y, timestamp); + BN256G2.G2Point memory Hm = BN256G2.hashToG2(encodedMessage, hashToG2Tag); + validateSignatureOrRevert(ids, blsSignature, Hm); + } + } + /// @notice Exits a service node by liquidating their node from the /// network rewarding the caller for maintaining the list. /// @@ -619,27 +644,9 @@ contract ServiceNodeRewards is Initializable, Ownable2StepUpgradeable, PausableU BLSSignatureParams calldata blsSignature, uint64[] memory ids ) external whenNotPaused whenStarted hasEnoughSigners(ids.length) { - bytes memory pubkeyBytes = BN256G1.getKeyForG1Point(blsPubkey); - uint64 serviceNodeID = serviceNodeIDs[pubkeyBytes]; - if (serviceNodeID == 0) - revert BLSPubkeyDoesNotExist(blsPubkey); - if (signatureTimestampHasExpired(timestamp)) { - revert SignatureExpired(serviceNodeID, timestamp, block.timestamp); - } - ServiceNode memory node = _serviceNodes[serviceNodeID]; - if (blsPubkey.X != node.blsPubkey.X || blsPubkey.Y != node.blsPubkey.Y) { - revert BLSPubkeyDoesNotMatch(serviceNodeID, blsPubkey); - } - if (block.timestamp < node.addedTimestamp + MINIMUM_LIQUIDATION_AGE) - revert LiquidationTooEarly(serviceNodeID, node.addedTimestamp, block.timestamp); - - // NOTE: Validate signature - { - bytes memory encodedMessage = abi.encodePacked(liquidateTag, blsPubkey.X, blsPubkey.Y, timestamp); - BN256G2.G2Point memory Hm = BN256G2.hashToG2(encodedMessage, hashToG2Tag); - validateSignatureOrRevert(ids, blsSignature, Hm); - } + (uint64 serviceNodeID, ServiceNode memory node) = _validateBLSExitWithSignature( + blsPubkey, timestamp, blsSignature, liquidateTag, ids); // Calculating how much liquidator is paid out emit ServiceNodeLiquidated(serviceNodeID, node.operator, node.blsPubkey); diff --git a/test/cpp/include/service_node_rewards/service_node_list.hpp b/test/cpp/include/service_node_rewards/service_node_list.hpp index 04c639b..3040b69 100644 --- a/test/cpp/include/service_node_rewards/service_node_list.hpp +++ b/test/cpp/include/service_node_rewards/service_node_list.hpp @@ -56,8 +56,16 @@ class ServiceNodeList { uint32_t chainID, const std::string& contractAddress, const std::vector& indices, - std::optional timestamp = std::nullopt); - std::tuple exitNodeFromIndices(uint64_t nodeID, uint32_t chainID, const std::string& contractAddress, const std::vector& indices); + std::optional timestamp = std::nullopt) { + return exitNodeFromIndices(nodeID, chainID, contractAddress, indices, timestamp, true); + } + std::tuple exitNodeFromIndices( + uint64_t nodeID, + uint32_t chainID, + const std::string& contractAddress, + const std::vector& indices, + std::optional timestamp = std::nullopt, + bool liquidate = false); std::string updateRewardsBalance(const std::string& address, uint64_t amount, uint32_t chainID, const std::string& contractAddress, const std::vector& service_node_ids); std::vector findNonSigners(const std::vector& indices); diff --git a/test/cpp/src/service_node_list.cpp b/test/cpp/src/service_node_list.cpp index e142ce8..75b792a 100644 --- a/test/cpp/src/service_node_list.cpp +++ b/test/cpp/src/service_node_list.cpp @@ -288,17 +288,18 @@ static uint64_t to_ts(std::chrono::system_clock::time_point tp) { std::chrono::duration_cast(tp.time_since_epoch()).count()); } -std::tuple ServiceNodeList::liquidateNodeFromIndices( +std::tuple ServiceNodeList::exitNodeFromIndices( uint64_t nodeID, uint32_t chainID, const std::string& contractAddress, const std::vector& service_node_ids, - std::optional timestamp) { + std::optional timestamp, + bool liquidate) { std::tuple result; auto& [pubkey, ts, sig] = result; pubkey = nodes[static_cast(findNodeIndex(nodeID))].getPublicKeyHex(); - std::string fullTag = buildTag(liquidateTag, chainID, contractAddress); + std::string fullTag = buildTag(liquidate ? liquidateTag : exitTag, chainID, contractAddress); ts = to_ts(timestamp.value_or(std::chrono::system_clock::now())); std::string message = "0x" + fullTag + pubkey + ethyl::utils::padTo32Bytes(ethyl::utils::decimalToHex(ts), ethyl::utils::PaddingDirection::LEFT); bls::Signature aggSig; @@ -311,20 +312,6 @@ std::tuple ServiceNodeList::liquidateNodeFro return result; } -std::tuple ServiceNodeList::exitNodeFromIndices(uint64_t nodeID, uint32_t chainID, const std::string& contractAddress, const std::vector& service_node_ids) { - std::string pubkey = nodes[static_cast(findNodeIndex(nodeID))].getPublicKeyHex(); - std::string fullTag = buildTag(exitTag, chainID, contractAddress); - auto timestamp = to_ts(std::chrono::system_clock::now()); - std::string message = "0x" + fullTag + pubkey + ethyl::utils::padTo32Bytes(ethyl::utils::decimalToHex(timestamp), ethyl::utils::PaddingDirection::LEFT); - bls::Signature aggSig; - aggSig.clear(); - std::vector messageBytes = ethyl::utils::fromHexString(message); - for(auto& service_node_id: service_node_ids) { - aggSig.add(nodes[static_cast(findNodeIndex(service_node_id))].blsSignHash(messageBytes, chainID, contractAddress)); - } - return std::make_tuple(pubkey, timestamp, utils::SignatureToHex(aggSig)); -} - std::string ServiceNodeList::updateRewardsBalance(const std::string& address, uint64_t amount, uint32_t chainID, const std::string& contractAddress, const std::vector& service_node_ids) { std::string rewardAddressOutput = address; if (rewardAddressOutput.substr(0, 2) == "0x") diff --git a/test/cpp/test/src/rewards_contract.cpp b/test/cpp/test/src/rewards_contract.cpp index 3a013da..76e204e 100644 --- a/test/cpp/test/src/rewards_contract.cpp +++ b/test/cpp/test/src/rewards_contract.cpp @@ -399,10 +399,36 @@ TEST_CASE( "Rewards Contract", "[ethereum]" ) { REQUIRE(rewards_contract.totalNodes() == 3); const uint64_t service_node_to_exit = snl.randomServiceNodeID(); const auto signers = snl.randomSigners(snl.nodes.size() - 1); - const auto [pubkey, timestamp, sig] = snl.exitNodeFromIndices(service_node_to_exit, config.CHAIN_ID, contract_address, signers); + auto [pubkey, timestamp, sig] = snl.exitNodeFromIndices(service_node_to_exit, config.CHAIN_ID, contract_address, signers); const auto non_signers = snl.findNonSigners(signers); tx = rewards_contract.exitBLSPublicKeyWithSignature(pubkey, timestamp, sig, non_signers); + + // Too soon to exit: + REQUIRE_THROWS(signer.sendTransaction(tx, seckey)); + + defaultProvider.evm_increaseTime(2h); + + // Exit signature timestamp expired: + REQUIRE_THROWS(signer.sendTransaction(tx, seckey)); + + std::tie(pubkey, timestamp, sig) = snl.exitNodeFromIndices( + service_node_to_exit, + config.CHAIN_ID, + contract_address, + signers, + std::chrono::system_clock::now() + 2h); + tx = rewards_contract.exitBLSPublicKeyWithSignature(pubkey, timestamp, sig, non_signers); + + // Leave request has not been submitted + REQUIRE_THROWS(signer.sendTransaction(tx, seckey)); + + tx = rewards_contract.initiateExitBLSPublicKey(service_node_to_exit); + signer.sendTransaction(tx, seckey); + + // Now we can actually exit: + tx = rewards_contract.exitBLSPublicKeyWithSignature(pubkey, timestamp, sig, non_signers); hash = signer.sendTransaction(tx, seckey); + REQUIRE(hash != ""); REQUIRE(defaultProvider.transactionSuccessful(hash)); REQUIRE(rewards_contract.totalNodes() == 2);