Implement MultiSignReserve amendment [RIPD-1647]:

Reduces the account reserve for a multisigning SignerList from (conditionally) 3 to 10 OwnerCounts to (unconditionally) 1 OwnerCount. Includes a transition process.
2025-12-06 17:27:52 +00:00 · 2018-07-13 12:34:26 -07:00
parent 3ce4dda5cb
commit 6572fc8e95
7 changed files with 390 additions and 208 deletions
--- a/src/test/app/MultiSign_test.cpp
+++ b/src/test/app/MultiSign_test.cpp
@@ -36,15 +36,23 @@ class MultiSign_test : public beast::unit_test::suite
    jtx::Account const spook {"spook", KeyType::ed25519};

 public:
-    void test_noReserve()
+    void test_noReserve (FeatureBitset features)
    {
+        testcase ("No Reserve");
+
        using namespace jtx;
-        Env env(*this);
+        Env env {*this, features};
        Account const alice {"alice", KeyType::secp256k1};

+        // The reserve required for a signer list changes with the passage
+        // of featureMultiSignReserve.  Make the required adjustments.
+        bool const reserve1 {features[featureMultiSignReserve]};
+
        // Pay alice enough to meet the initial reserve, but not enough to
        // meet the reserve for a SignerListSet.
-        env.fund(XRP(200), alice);
+        auto const fee = env.current()->fees().base;
+        auto const smallSignersReserve = reserve1 ? XRP(250) : XRP(350);
+        env.fund(smallSignersReserve - drops (1), alice);
        env.close();
        env.require (owners (alice, 0));

@@ -56,27 +64,32 @@ public:
            env.require (owners (alice, 0));

            // Fund alice enough to set the signer list, then attach signers.
-            env(pay(env.master, alice, XRP(151)));
+            env(pay(env.master, alice, fee + drops (1)));
            env.close();
            env(smallSigners);
            env.close();
-            env.require (owners (alice, 3));
+            env.require (owners (alice, reserve1 ? 1 : 3));
        }
        {
+            // Pay alice enough to almost make the reserve for the biggest
+            // possible list.
+            auto const addReserveBigSigners = reserve1 ? XRP(0) : XRP(350);
+            env(pay(env.master, alice, addReserveBigSigners + fee - drops(1)));
+
            // Replace with the biggest possible signer list.  Should fail.
            Json::Value bigSigners = signers(alice, 1, {
                { bogie, 1 }, { demon, 1 }, { ghost, 1 }, { haunt, 1 },
                { jinni, 1 }, { phase, 1 }, { shade, 1 }, { spook, 1 }});
            env(bigSigners, ter(tecINSUFFICIENT_RESERVE));
            env.close();
-            env.require (owners (alice, 3));
+            env.require (owners (alice, reserve1 ? 1 : 3));

-            // Fund alice and succeed.
-            env(pay(env.master, alice, XRP(350)));
+            // Fund alice one more drop (plus the fee) and succeed.
+            env(pay(env.master, alice, fee + drops(1)));
            env.close();
            env(bigSigners);
            env.close();
-            env.require (owners (alice, 10));
+            env.require (owners (alice, reserve1 ? 1 : 10));
        }
        // Remove alice's signer list and get the owner count back.
        env(signers(alice, jtx::none));
@@ -84,10 +97,12 @@ public:
        env.require (owners (alice, 0));
    }

-    void test_signerListSet()
+    void test_signerListSet (FeatureBitset features)
    {
+        testcase ("SignerListSet");
+
        using namespace jtx;
-        Env env(*this);
+        Env env {*this, features};
        Account const alice {"alice", KeyType::ed25519};
        env.fund(XRP(1000), alice);

@@ -129,10 +144,12 @@ public:
        env.require (owners (alice, 0));
    }

-    void test_phantomSigners()
+    void test_phantomSigners (FeatureBitset features)
    {
+        testcase ("Phantom Signers");
+
        using namespace jtx;
-        Env env(*this);
+        Env env {*this, features};
        Account const alice {"alice", KeyType::ed25519};
        env.fund(XRP(1000), alice);
        env.close();
@@ -140,7 +157,7 @@ public:
        // Attach phantom signers to alice and use them for a transaction.
        env(signers(alice, 1, {{bogie, 1}, {demon, 1}}));
        env.close();
-        env.require (owners (alice, 4));
+        env.require (owners (alice, features[featureMultiSignReserve] ? 1 : 4));

        // This should work.
        auto const baseFee = env.current()->fees().base;
@@ -188,20 +205,22 @@ public:
    }

    void
-    test_enablement()
+    test_enablement (FeatureBitset features)
    {
+        testcase ("Enablement");
+
        using namespace jtx;
        Env env(*this, envconfig([](std::unique_ptr<Config> cfg)
            {
                cfg->loadFromString ("[" SECTION_SIGNING_SUPPORT "]\ntrue");
                return cfg;
-            }), FeatureBitset{});
+            }), features - featureMultiSign);

        Account const alice {"alice", KeyType::ed25519};
        env.fund(XRP(1000), alice);
        env.close();

-        // NOTE: These six tests will fail when multisign is default enabled.
+        // NOTE: These six tests will fail if multisign is enabled.
        env(signers(alice, 1, {{bogie, 1}}), ter(temDISABLED));
        env.close();
        env.require (owners (alice, 0));
@@ -235,10 +254,12 @@ public:
        }
    }

-    void test_fee ()
+    void test_fee (FeatureBitset features)
    {
+        testcase ("Fee");
+
        using namespace jtx;
-        Env env(*this);
+        Env env {*this, features};
        Account const alice {"alice", KeyType::ed25519};
        env.fund(XRP(1000), alice);
        env.close();
@@ -247,7 +268,7 @@ public:
        env(signers(alice, 1, {{bogie, 1}, {demon, 1}, {ghost, 1}, {haunt, 1},
            {jinni, 1}, {phase, 1}, {shade, 1}, {spook, 1}}));
        env.close();
-        env.require (owners (alice, 10));
+        env.require (owners(alice, features[featureMultiSignReserve] ? 1 : 10));

        // This should work.
        auto const baseFee = env.current()->fees().base;
@@ -285,10 +306,12 @@ public:
        BEAST_EXPECT(env.seq(alice) == aliceSeq);
    }

-    void test_misorderedSigners()
+    void test_misorderedSigners (FeatureBitset features)
    {
+        testcase ("Misordered Signers");
+
        using namespace jtx;
-        Env env(*this);
+        Env env {*this, features};
        Account const alice {"alice", KeyType::ed25519};
        env.fund(XRP(1000), alice);
        env.close();
@@ -297,7 +320,7 @@ public:
        // Make sure the transaction fails if they are not.
        env(signers(alice, 1, {{bogie, 1}, {demon, 1}}));
        env.close();
-        env.require (owners(alice, 4));
+        env.require (owners (alice, features[featureMultiSignReserve] ? 1 : 4));

        msig phantoms {bogie, demon};
        std::reverse (phantoms.signers.begin(), phantoms.signers.end());
@@ -307,10 +330,12 @@ public:
        BEAST_EXPECT(env.seq(alice) == aliceSeq);
    }

-    void test_masterSigners()
+    void test_masterSigners (FeatureBitset features)
    {
+        testcase ("Master Signers");
+
        using namespace jtx;
-        Env env(*this);
+        Env env {*this, features};
        Account const alice {"alice", KeyType::ed25519};
        Account const becky {"becky", KeyType::secp256k1};
        Account const cheri {"cheri", KeyType::ed25519};
@@ -330,7 +355,7 @@ public:
        //Attach signers to alice
        env(signers(alice, 4, {{becky, 3}, {cheri, 4}}), sig (alice));
        env.close();
-        env.require (owners (alice, 4));
+        env.require (owners (alice, features[featureMultiSignReserve] ? 1 : 4));

        // Attempt a multisigned transaction that meets the quorum.
        auto const baseFee = env.current()->fees().base;
@@ -359,10 +384,12 @@ public:
        BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
    }

-    void test_regularSigners()
+    void test_regularSigners (FeatureBitset features)
    {
+        testcase ("Regular Signers");
+
        using namespace jtx;
-        Env env(*this);
+        Env env {*this, features};
        Account const alice {"alice", KeyType::secp256k1};
        Account const becky {"becky", KeyType::ed25519};
        Account const cheri {"cheri", KeyType::secp256k1};
@@ -417,14 +444,16 @@ public:
        BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
    }

-    void test_regularSignersUsingSubmitMulti()
+    void test_regularSignersUsingSubmitMulti (FeatureBitset features)
    {
+        testcase ("Regular Signers Using submit_multisigned");
+
        using namespace jtx;
        Env env(*this, envconfig([](std::unique_ptr<Config> cfg)
            {
                cfg->loadFromString ("[" SECTION_SIGNING_SUPPORT "]\ntrue");
                return cfg;
-            }));
+            }), features);
        Account const alice {"alice", KeyType::secp256k1};
        Account const becky {"becky", KeyType::ed25519};
        Account const cheri {"cheri", KeyType::secp256k1};
@@ -624,10 +653,12 @@ public:
        }
    }

-    void test_heterogeneousSigners()
+    void test_heterogeneousSigners (FeatureBitset features)
    {
+        testcase ("Heterogenious Signers");
+
        using namespace jtx;
-        Env env(*this);
+        Env env {*this, features};
        Account const alice {"alice", KeyType::secp256k1};
        Account const becky {"becky", KeyType::ed25519};
        Account const cheri {"cheri", KeyType::secp256k1};
@@ -656,7 +687,7 @@ public:
        env(signers(alice, 1,
            {{becky, 1}, {cheri, 1}, {daria, 1}, {jinni, 1}}), sig (alie));
        env.close();
-        env.require (owners (alice, 6));
+        env.require (owners (alice, features[featureMultiSignReserve] ? 1 : 6));

        // Each type of signer should succeed individually.
        auto const baseFee = env.current()->fees().base;
@@ -696,7 +727,7 @@ public:
        env(signers(alice, 0x3FFFC, {{becky, 0xFFFF},
            {cheri, 0xFFFF}, {daria, 0xFFFF}, {jinni, 0xFFFF}}), sig (alie));
        env.close();
-        env.require (owners (alice, 6));
+        env.require (owners (alice, features[featureMultiSignReserve] ? 1 : 6));

        aliceSeq = env.seq (alice);
        env(noop(alice), fee(9 * baseFee),
@@ -716,7 +747,7 @@ public:
            {daria, 0xFFFF}, {haunt, 0xFFFF}, {jinni, 0xFFFF},
            {phase, 0xFFFF}, {shade, 0xFFFF}, {spook, 0xFFFF}}), sig (alie));
        env.close();
-        env.require (owners (alice, 10));
+        env.require (owners(alice, features[featureMultiSignReserve] ? 1 : 10));

        aliceSeq = env.seq (alice);
        env(noop(alice), fee(9 * baseFee), msig(becky, msig::Reg{cheri, cher},
@@ -739,10 +770,12 @@ public:

    // We want to always leave an account signable.  Make sure the that we
    // disallow removing the last way a transaction may be signed.
-    void test_keyDisable()
+    void test_keyDisable (FeatureBitset features)
    {
+        testcase ("Key Disable");
+
        using namespace jtx;
-        Env env(*this);
+        Env env {*this, features};
        Account const alice {"alice", KeyType::ed25519};
        env.fund(XRP(1000), alice);

@@ -814,10 +847,12 @@ public:

    // Verify that the first regular key can be made for free using the
    // master key, but not when multisigning.
-    void test_regKey()
+    void test_regKey (FeatureBitset features)
    {
+        testcase ("Regular Key");
+
        using namespace jtx;
-        Env env(*this);
+        Env env {*this, features};
        Account const alice {"alice", KeyType::secp256k1};
        env.fund(XRP(1000), alice);

@@ -846,10 +881,12 @@ public:
    }

    // See if every kind of transaction can be successfully multi-signed.
-    void test_txTypes()
+    void test_txTypes (FeatureBitset features)
    {
+        testcase ("Transaction Types");
+
        using namespace jtx;
-        Env env(*this);
+        Env env {*this, features};
        Account const alice {"alice", KeyType::secp256k1};
        Account const becky {"becky", KeyType::ed25519};
        Account const zelda {"zelda", KeyType::secp256k1};
@@ -866,7 +903,8 @@ public:
        // Attach signers to alice.
        env(signers(alice, 2, {{becky, 1}, {bogie, 1}}), sig (alie));
        env.close();
-        env.require (owners (alice, 4));
+        int const signerListOwners {features[featureMultiSignReserve] ? 1 : 4};
+        env.require (owners (alice, signerListOwners + 0));

        // Multisign a ttPAYMENT.
        auto const baseFee = env.current()->fees().base;
@@ -893,7 +931,7 @@ public:
        env(trust("alice", USD(100)),
            msig(becky, bogie), fee(3 * baseFee), require (lines("alice", 1)));
        env.close();
-        env.require (owners (alice, 5));
+        env.require (owners (alice, signerListOwners + 1));

        // Multisign a ttOFFER_CREATE transaction.
        env(pay(gw, alice, USD(50)));
@@ -905,7 +943,7 @@ public:
        env(offer(alice, XRP(50), USD(50)),
            msig (becky, bogie), fee(3 * baseFee));
        env.close();
-        env.require(owners(alice, 6));
+        env.require (owners (alice, signerListOwners + 2));

        // Now multisign a ttOFFER_CANCEL canceling the offer we just created.
        {
@@ -918,22 +956,24 @@ public:
                msig (becky, bogie), fee(3 * baseFee));
            env.close();
            BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
-            env.require(owners(alice, 5));
+            env.require(owners(alice, signerListOwners + 1));
        }

        // Multisign a ttSIGNER_LIST_SET.
        env(signers(alice, 3, {{becky, 1}, {bogie, 1}, {demon, 1}}),
            msig (becky, bogie), fee(3 * baseFee));
        env.close();
-        env.require (owners (alice, 6));
+        env.require (owners (alice, features[featureMultiSignReserve] ? 2 : 6));
    }

-    void test_badSignatureText()
+    void test_badSignatureText (FeatureBitset features)
    {
+        testcase ("Bad Signature Text");
+
        // Verify that the text returned for signature failures is correct.
        using namespace jtx;

-        Env env(*this);
+        Env env {*this, features};

        // lambda that submits an STTx and returns the resulting JSON.
        auto submitSTTx = [&env] (STTx const& stx)
@@ -1064,10 +1104,12 @@ public:
        }
    }

-    void test_noMultiSigners()
+    void test_noMultiSigners (FeatureBitset features)
    {
+        testcase ("No Multisigners");
+
        using namespace jtx;
-        Env env {*this};
+        Env env {*this, features};
        Account const alice {"alice", KeyType::ed25519};
        Account const becky {"becky", KeyType::secp256k1};
        env.fund(XRP(1000), alice, becky);
@@ -1077,15 +1119,17 @@ public:
        env(noop(alice), msig(becky, demon), fee(3 * baseFee), ter(tefNOT_MULTI_SIGNING));
    }

-    void test_multisigningMultisigner()
+    void test_multisigningMultisigner (FeatureBitset features)
    {
+        testcase ("Multisigning multisigner");
+
        // Set up a signer list where one of the signers has both the
        // master disabled and no regular key (because that signer is
        // exclusively multisigning).  That signer should no longer be
        // able to successfully sign the signer list.

        using namespace jtx;
-        Env env (*this);
+        Env env {*this, features};
        Account const alice {"alice", KeyType::ed25519};
        Account const becky {"becky", KeyType::secp256k1};
        env.fund (XRP(1000), alice, becky);
@@ -1143,8 +1187,10 @@ public:
        env.close();
    }

-    void test_signForHash()
+    void test_signForHash (FeatureBitset features)
    {
+        testcase ("sign_for Hash");
+
        // Make sure that the "hash" field returned by the "sign_for" RPC
        // command matches the hash returned when that command is sent
        // through "submit_multisigned".  Make sure that hash also locates
@@ -1156,7 +1202,7 @@ public:
            {
                cfg->loadFromString ("[" SECTION_SIGNING_SUPPORT "]\ntrue");
                return cfg;
-            }));
+            }), features);
        env.fund (XRP(1000), alice);
        env.close();

@@ -1221,25 +1267,104 @@ public:
            [sfTransactionResult.jsonName].asString() == "tesSUCCESS");
    }

+    void test_amendmentTransition ()
+    {
+        testcase ("Amendment Transition");
+
+        // The OwnerCount associated with a SignerList changes once the
+        // featureMultiSignReserve amendment goes live.  Create a couple
+        // of signer lists before and after the amendment goes live and
+        // verify that the OwnerCount is managed properly for all of them.
+        using namespace jtx;
+        Account const alice {"alice", KeyType::secp256k1};
+        Account const becky {"becky", KeyType::ed25519};
+        Account const cheri {"cheri", KeyType::secp256k1};
+        Account const daria {"daria", KeyType::ed25519};
+
+        Env env {*this, supported_amendments() - featureMultiSignReserve};
+        env.fund (XRP(1000), alice, becky, cheri, daria);
+        env.close();
+
+        // Give alice and becky signer lists before the amendment goes live.
+        env (signers (alice, 1, {{bogie, 1}}));
+        env (signers (becky, 1, {{bogie, 1}, {demon, 1}, {ghost, 1},
+            {haunt, 1}, {jinni, 1}, {phase, 1}, {shade, 1}, {spook, 1}}));
+        env.close();
+
+        env.require (owners (alice, 3));
+        env.require (owners (becky, 10));
+
+        // Enable the amendment.
+        env.enableFeature (featureMultiSignReserve);
+        env.close();
+
+        // Give cheri and daria signer lists after the amendment goes live.
+        env (signers (cheri, 1, {{bogie, 1}}));
+        env (signers (daria, 1, {{bogie, 1}, {demon, 1}, {ghost, 1},
+            {haunt, 1}, {jinni, 1}, {phase, 1}, {shade, 1}, {spook, 1}}));
+        env.close();
+
+        env.require (owners (alice, 3));
+        env.require (owners (becky, 10));
+        env.require (owners (cheri, 1));
+        env.require (owners (daria, 1));
+
+        // Delete becky's signer list; her OwnerCount should drop to zero.
+        // Replace alice's signer list; her OwnerCount should drop to one.
+        env (signers (becky, jtx::none));
+        env (signers (alice, 1, {{bogie, 1}, {demon, 1}, {ghost, 1},
+            {haunt, 1}, {jinni, 1}, {phase, 1}, {shade, 1}, {spook, 1}}));
+        env.close();
+
+        env.require (owners (alice, 1));
+        env.require (owners (becky, 0));
+        env.require (owners (cheri, 1));
+        env.require (owners (daria, 1));
+
+        // Delete the three remaining signer lists.  Everybody's OwnerCount
+        // should now be zero.
+        env (signers (alice, jtx::none));
+        env (signers (cheri, jtx::none));
+        env (signers (daria, jtx::none));
+        env.close();
+
+        env.require (owners (alice, 0));
+        env.require (owners (becky, 0));
+        env.require (owners (cheri, 0));
+        env.require (owners (daria, 0));
+    }
+
+    void testAll(FeatureBitset features)
+    {
+        test_noReserve (features);
+        test_signerListSet (features);
+        test_phantomSigners (features);
+        test_enablement (features);
+        test_fee (features);
+        test_misorderedSigners (features);
+        test_masterSigners (features);
+        test_regularSigners (features);
+        test_regularSignersUsingSubmitMulti (features);
+        test_heterogeneousSigners (features);
+        test_keyDisable (features);
+        test_regKey (features);
+        test_txTypes (features);
+        test_badSignatureText (features);
+        test_noMultiSigners (features);
+        test_multisigningMultisigner (features);
+        test_signForHash (features);
+    }
+
    void run() override
    {
-        test_noReserve();
-        test_signerListSet();
-        test_phantomSigners();
-        test_enablement();
-        test_fee();
-        test_misorderedSigners();
-        test_masterSigners();
-        test_regularSigners();
-        test_regularSignersUsingSubmitMulti();
-        test_heterogeneousSigners();
-        test_keyDisable();
-        test_regKey();
-        test_txTypes();
-        test_badSignatureText();
-        test_noMultiSigners();
-        test_multisigningMultisigner();
-        test_signForHash();
+        using namespace jtx;
+        auto const all = supported_amendments();
+
+        // The reserve required on a signer list changes based on.
+        // featureMultiSignReserve.  Test both with and without.
+        testAll (all - featureMultiSignReserve);
+        testAll (all | featureMultiSignReserve);
+        test_amendmentTransition();
    }
 };