Add 2h minimum liquidation age

This adds a 2-hour minimum age before a node may be liquidated. This matches the 2h decommission credit all nodes start with (and so, in effect, we should always expect any legitimate deregistration to be *at least* 2h after adding the BLS key). This buffer is needed to avoid a potential sort of "front-running" deregistration attack where a malicious entity could observe an incoming registration, obtain a liquidation signature from the Oxen SN network before that registration hits the network, and then immediately submit to the liquidation to effective block new registrations from the network. A considered alternative to this approach was to have oxend only sign liquidation requests that are in the recently-removed-nodes list, and while that work mitigate the attack above, it would introduce a new problem where a registration that oxend failed to process for whatever reason (e.g. invalid ed25519 key, or some other unforseen error) would result in a BLS registration in the contract that was permanently unremovable.
oxen-io · Oct 22, 2024 · de5f98b · de5f98b
1 parent e81a777
commit de5f98b
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 10 deletions.
diff --git a/contracts/ServiceNodeRewards.sol b/contracts/ServiceNodeRewards.sol
@@ -28,6 +28,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;
 
     uint64 public nextServiceNodeID;
     uint256 public totalNodes;
@@ -205,6 +211,7 @@ contract ServiceNodeRewards is Initializable, Ownable2StepUpgradeable, PausableU
     error InvalidBLSSignature(BN256G1.G1Point aggPubkey);
     error InvalidBLSProofOfPossession();
     error LeaveRequestTooEarly(uint64 serviceNodeID, uint256 timestamp, uint256 currenttime);
+    error LiquidationTooEarly(uint64 serviceNodeID, uint256 addedTimestamp, uint256 currenttime);
     error LiquidatorRewardsTooLow();
     error MaxContributorsExceeded();
     error MaxClaimExceeded();
@@ -572,7 +579,7 @@ contract ServiceNodeRewards is Initializable, Ownable2StepUpgradeable, PausableU
     /// approve the liquidation by aggregating a valid BLS signature. The nodes
     /// will only provide this signature if the consensus rules permit the node
     /// to be forcibly removed (e.g. the node was deregistered by consensus in
-    /// Oxen's state-chain).
+    /// Oxen's state-chain, or does not exist on the Oxen chain).
     ///
     /// @param blsPubkey 64 byte BLS public key for the service node to be
     /// removed.
@@ -597,6 +604,8 @@ contract ServiceNodeRewards is Initializable, Ownable2StepUpgradeable, PausableU
         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
         {

diff --git a/test/cpp/include/service_node_rewards/service_node_list.hpp b/test/cpp/include/service_node_rewards/service_node_list.hpp
@@ -14,6 +14,8 @@
 #undef MCLBN_NO_AUTOLINK
 #pragma GCC diagnostic pop
 
+#include <chrono>
+#include <optional>
 #include <string>
 #include <vector>
 #include <span>
@@ -49,7 +51,12 @@ class ServiceNodeList {
     std::string aggregateSignatures(const std::string& message, uint32_t chainID, std::string_view contractAddress);
     std::string aggregateSignaturesFromIndices(const std::string& message, const std::vector<int64_t>& indices, uint32_t chainID, std::string_view contractAddress);
 
-    std::tuple<std::string, uint64_t, std::string> liquidateNodeFromIndices(uint64_t nodeID, uint32_t chainID, const std::string& contractAddress, const std::vector<uint64_t>& indices);
+    std::tuple<std::string, uint64_t, std::string> liquidateNodeFromIndices(
+            uint64_t nodeID,
+            uint32_t chainID,
+            const std::string& contractAddress,
+            const std::vector<uint64_t>& indices,
+            std::optional<std::chrono::system_clock::time_point> timestamp = std::nullopt);
     std::tuple<std::string, uint64_t, std::string> removeNodeFromIndices(uint64_t nodeID, uint32_t chainID, const std::string& contractAddress, const std::vector<uint64_t>& indices);
     std::string updateRewardsBalance(const std::string& address, uint64_t amount, uint32_t chainID, const std::string& contractAddress, const std::vector<uint64_t>& service_node_ids);
 

diff --git a/test/cpp/src/service_node_list.cpp b/test/cpp/src/service_node_list.cpp
@@ -283,24 +283,38 @@ uint64_t ServiceNodeList::randomServiceNodeID() {
     return serviceNodeIDs[0];
 }
 
-std::tuple<std::string, uint64_t, std::string> ServiceNodeList::liquidateNodeFromIndices(uint64_t nodeID, uint32_t chainID, const std::string& contractAddress, const std::vector<uint64_t>& service_node_ids) {
-    std::string pubkey = nodes[static_cast<size_t>(findNodeIndex(nodeID))].getPublicKeyHex();
+static uint64_t to_ts(std::chrono::system_clock::time_point tp) {
+    return static_cast<uint64_t>(
+            std::chrono::duration_cast<std::chrono::seconds>(tp.time_since_epoch()).count());
+}
+
+std::tuple<std::string, uint64_t, std::string> ServiceNodeList::liquidateNodeFromIndices(
+        uint64_t nodeID,
+        uint32_t chainID,
+        const std::string& contractAddress,
+        const std::vector<uint64_t>& service_node_ids,
+        std::optional<std::chrono::system_clock::time_point> timestamp) {
+    std::tuple<std::string, uint64_t, std::string> result;
+    auto& [pubkey, ts, sig] = result;
+
+    pubkey = nodes[static_cast<size_t>(findNodeIndex(nodeID))].getPublicKeyHex();
     std::string fullTag = buildTag(liquidateTag, chainID, contractAddress);
-    auto timestamp = static_cast<uint64_t>(std::chrono::duration_cast<std::chrono::seconds>(std::chrono::system_clock::now().time_since_epoch()).count());
-    std::string message = "0x" + fullTag + pubkey + ethyl::utils::padTo32Bytes(ethyl::utils::decimalToHex(timestamp), ethyl::utils::PaddingDirection::LEFT);
+    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;
     aggSig.clear();
     std::vector<uint8_t> messageBytes = ethyl::utils::fromHexString<uint8_t>(message);
     for(auto& service_node_id: service_node_ids) {
         aggSig.add(nodes[static_cast<size_t>(findNodeIndex(service_node_id))].blsSignHash(messageBytes, chainID, contractAddress));
     }
-    return std::make_tuple(pubkey, timestamp, utils::SignatureToHex(aggSig));
+    sig = utils::SignatureToHex(aggSig);
+    return result;
 }
 
 std::tuple<std::string, uint64_t, std::string> ServiceNodeList::removeNodeFromIndices(uint64_t nodeID, uint32_t chainID, const std::string& contractAddress, const std::vector<uint64_t>& service_node_ids) {
     std::string pubkey = nodes[static_cast<size_t>(findNodeIndex(nodeID))].getPublicKeyHex();
     std::string fullTag = buildTag(removalTag, chainID, contractAddress);
-    auto timestamp = static_cast<uint64_t>(std::chrono::duration_cast<std::chrono::seconds>(std::chrono::system_clock::now().time_since_epoch()).count());
+    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();

diff --git a/test/cpp/test/src/rewards_contract.cpp b/test/cpp/test/src/rewards_contract.cpp
@@ -198,10 +198,27 @@ TEST_CASE( "Rewards Contract", "[ethereum]" ) {
         REQUIRE(rewards_contract.serviceNodesLength() == 3);
         const uint64_t service_node_to_remove = snl.randomServiceNodeID();
         const auto signers = snl.randomSigners(snl.nodes.size());
-        const auto [pubkey, timestamp, sig] = snl.liquidateNodeFromIndices(service_node_to_remove, config.CHAIN_ID, contract_address, signers);
+        auto [pubkey, timestamp, sig] = snl.liquidateNodeFromIndices(service_node_to_remove, config.CHAIN_ID, contract_address, signers);
         const auto non_signers = snl.findNonSigners(signers);
         tx = rewards_contract.liquidateBLSPublicKeyWithSignature(pubkey, timestamp, sig, non_signers);
+
+        // Too soon to liquidate:
+        REQUIRE_THROWS(signer.sendTransaction(tx, seckey));
+
+        defaultProvider.evm_increaseTime(2h);
+
+        // Liquidation signature timestamp expired:
+        REQUIRE_THROWS(signer.sendTransaction(tx, seckey));
+
+        std::tie(pubkey, timestamp, sig) = snl.liquidateNodeFromIndices(
+                service_node_to_remove,
+                config.CHAIN_ID,
+                contract_address,
+                signers,
+                std::chrono::system_clock::now() + 2h);
+        tx = rewards_contract.liquidateBLSPublicKeyWithSignature(pubkey, timestamp, sig, non_signers);
         hash = signer.sendTransaction(tx, seckey);
+
         REQUIRE(hash != "");
         REQUIRE(defaultProvider.transactionSuccessful(hash));
         REQUIRE(rewards_contract.serviceNodesLength() == 2);
@@ -224,7 +241,9 @@ TEST_CASE( "Rewards Contract", "[ethereum]" ) {
         REQUIRE(rewards_contract.serviceNodesLength() == 3);
         const uint64_t service_node_to_remove = snl.randomServiceNodeID();
         const auto signers = snl.randomSigners(snl.nodes.size() - 1);
-        const auto [pubkey, timestamp, sig] = snl.liquidateNodeFromIndices(service_node_to_remove, config.CHAIN_ID, contract_address, signers);
+        defaultProvider.evm_increaseTime(2h);
+        const auto [pubkey, timestamp, sig] = snl.liquidateNodeFromIndices(service_node_to_remove, config.CHAIN_ID, contract_address, signers,
+                std::chrono::system_clock::now() + 2h);
         const auto non_signers = snl.findNonSigners(signers);
         tx = rewards_contract.liquidateBLSPublicKeyWithSignature(pubkey, timestamp, sig, non_signers);
         hash = signer.sendTransaction(tx, seckey);