diff --git a/include/xrpl/protocol/STTx.h b/include/xrpl/protocol/STTx.h index 6544d72cd0..716881d910 100644 --- a/include/xrpl/protocol/STTx.h +++ b/include/xrpl/protocol/STTx.h @@ -31,17 +31,8 @@ class STTx final : public STObject, public CountedObject TxType tx_type_; public: - static std::size_t const minMultiSigners = 1; - - // if rules are not supplied then the largest possible value is returned - static std::size_t - maxMultiSigners(Rules const* rules = 0) - { - if (rules && !rules->enabled(featureExpandedSignerList)) - return 8; - - return 32; - } + static constexpr std::size_t minMultiSigners = 1; + static constexpr std::size_t maxMultiSigners = 32; STTx() = delete; STTx(STTx const& other) = default; diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index f38793cb9e..855a714f69 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -65,13 +65,11 @@ XRPL_FIX (UniversalNumber, Supported::yes, VoteBehavior::DefaultNo XRPL_FEATURE(XRPFees, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(DisallowIncoming, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (RemoveNFTokenAutoTrustLine, Supported::yes, VoteBehavior::DefaultYes) -XRPL_FEATURE(ExpandedSignerList, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(FlowSortStrands, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(TicketBatch, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(NegativeUNL, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(RequireFullyCanonicalSig, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(DeletableAccounts, Supported::yes, VoteBehavior::DefaultYes) -XRPL_FEATURE(MultiSignReserve, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(Checks, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(Flow, Supported::yes, VoteBehavior::DefaultYes) @@ -126,11 +124,13 @@ XRPL_RETIRE_FEATURE(DepositAuth) XRPL_RETIRE_FEATURE(DepositPreauth) XRPL_RETIRE_FEATURE(Escrow) XRPL_RETIRE_FEATURE(EnforceInvariants) +XRPL_RETIRE_FEATURE(ExpandedSignerList) XRPL_RETIRE_FEATURE(FeeEscalation) XRPL_RETIRE_FEATURE(FlowCross) XRPL_RETIRE_FEATURE(HardenedValidations) XRPL_RETIRE_FEATURE(ImmediateOfferKilled) XRPL_RETIRE_FEATURE(MultiSign) +XRPL_RETIRE_FEATURE(MultiSignReserve) XRPL_RETIRE_FEATURE(NonFungibleTokensV1_1) XRPL_RETIRE_FEATURE(PayChan) XRPL_RETIRE_FEATURE(SortedDirectories) diff --git a/src/libxrpl/protocol/STTx.cpp b/src/libxrpl/protocol/STTx.cpp index f868ab0711..dbcf1f304e 100644 --- a/src/libxrpl/protocol/STTx.cpp +++ b/src/libxrpl/protocol/STTx.cpp @@ -476,7 +476,7 @@ multiSignHelper( // There are well known bounds that the number of signers must be within. if (signers.size() < STTx::minMultiSigners || - signers.size() > STTx::maxMultiSigners(&rules)) + signers.size() > STTx::maxMultiSigners) return Unexpected("Invalid Signers array size."); // Signers must be in sorted order by AccountID. diff --git a/src/test/app/AMMExtended_test.cpp b/src/test/app/AMMExtended_test.cpp index 6b09ea0ce5..080b392ebe 100644 --- a/src/test/app/AMMExtended_test.cpp +++ b/src/test/app/AMMExtended_test.cpp @@ -3534,8 +3534,7 @@ private: // Attach signers to alice. env(signers(alice, 2, {{becky, 1}, {bogie, 1}}), sig(alie)); env.close(); - int const signerListOwners{features[featureMultiSignReserve] ? 2 : 5}; - env.require(owners(alice, signerListOwners + 0)); + env.require(owners(alice, 2)); msig const ms{becky, bogie}; @@ -3795,20 +3794,13 @@ private: void testMultisign() { - using namespace jtx; - auto const all = testable_amendments(); - - testTxMultisign( - all - featureMultiSignReserve - featureExpandedSignerList); - testTxMultisign(all - featureExpandedSignerList); - testTxMultisign(all); + testTxMultisign(jtx::testable_amendments()); } void testPayStrand() { - using namespace jtx; - auto const all = testable_amendments(); + auto const all = jtx::testable_amendments(); testToStrand(all); testRIPD1373(all); diff --git a/src/test/app/Check_test.cpp b/src/test/app/Check_test.cpp index a9534bf5f9..61e20a0098 100644 --- a/src/test/app/Check_test.cpp +++ b/src/test/app/Check_test.cpp @@ -963,14 +963,8 @@ class Check_test : public beast::unit_test::suite } // Use a regular key and also multisign to cash a check. - // featureMultiSignReserve changes the reserve on a SignerList, so - // check both before and after. - for (auto const& testFeatures : - {features - featureMultiSignReserve, - features | featureMultiSignReserve}) { - Env env{*this, testFeatures}; - + Env env{*this, features}; env.fund(XRP(1000), gw, alice, bob); env.close(); @@ -999,11 +993,7 @@ class Check_test : public beast::unit_test::suite env(signers(bob, 2, {{bogie, 1}, {demon, 1}}), sig(bobby)); env.close(); - // If featureMultiSignReserve is enabled then bob's signer list - // has an owner count of 1, otherwise it's 4. - int const signersCount = { - testFeatures[featureMultiSignReserve] ? 1 : 4}; - BEAST_EXPECT(ownerCount(env, bob) == signersCount + 1); + BEAST_EXPECT(ownerCount(env, bob) == 2); // bob uses his regular key to cash a check. env(check::cash(bob, chkId1, (USD(1))), sig(bobby)); @@ -1013,7 +1003,7 @@ class Check_test : public beast::unit_test::suite BEAST_EXPECT(checksOnAccount(env, alice).size() == 1); BEAST_EXPECT(checksOnAccount(env, bob).size() == 1); BEAST_EXPECT(ownerCount(env, alice) == 2); - BEAST_EXPECT(ownerCount(env, bob) == signersCount + 1); + BEAST_EXPECT(ownerCount(env, bob) == 2); // bob uses multisigning to cash a check. XRPAmount const baseFeeDrops{env.current()->fees().base}; @@ -1026,7 +1016,7 @@ class Check_test : public beast::unit_test::suite BEAST_EXPECT(checksOnAccount(env, alice).size() == 0); BEAST_EXPECT(checksOnAccount(env, bob).size() == 0); BEAST_EXPECT(ownerCount(env, alice) == 1); - BEAST_EXPECT(ownerCount(env, bob) == signersCount + 1); + BEAST_EXPECT(ownerCount(env, bob) == 2); } } @@ -1605,13 +1595,8 @@ class Check_test : public beast::unit_test::suite Account const zoe{"zoe"}; IOU const USD{gw["USD"]}; - // featureMultiSignReserve changes the reserve on a SignerList, so - // check both before and after. - for (auto const& testFeatures : - {features - featureMultiSignReserve, - features | featureMultiSignReserve}) { - Env env{*this, testFeatures}; + Env env{*this, features}; env.fund(XRP(1000), gw, alice, bob, zoe); env.close(); @@ -1729,16 +1714,11 @@ class Check_test : public beast::unit_test::suite env(signers(alice, 2, {{bogie, 1}, {demon, 1}}), sig(alie)); env.close(); - // If featureMultiSignReserve is enabled then alices's signer list - // has an owner count of 1, otherwise it's 4. - int const signersCount{ - testFeatures[featureMultiSignReserve] ? 1 : 4}; - // alice uses her regular key to cancel a check. env(check::cancel(alice, chkIdReg), sig(alie)); env.close(); BEAST_EXPECT(checksOnAccount(env, alice).size() == 3); - BEAST_EXPECT(ownerCount(env, alice) == signersCount + 3); + BEAST_EXPECT(ownerCount(env, alice) == 4); // alice uses multisigning to cancel a check. XRPAmount const baseFeeDrops{env.current()->fees().base}; @@ -1747,18 +1727,18 @@ class Check_test : public beast::unit_test::suite fee(3 * baseFeeDrops)); env.close(); BEAST_EXPECT(checksOnAccount(env, alice).size() == 2); - BEAST_EXPECT(ownerCount(env, alice) == signersCount + 2); + BEAST_EXPECT(ownerCount(env, alice) == 3); // Creator and destination cancel the remaining unexpired checks. env(check::cancel(alice, chkId3), sig(alice)); env.close(); BEAST_EXPECT(checksOnAccount(env, alice).size() == 1); - BEAST_EXPECT(ownerCount(env, alice) == signersCount + 1); + BEAST_EXPECT(ownerCount(env, alice) == 2); env(check::cancel(bob, chkIdNotExp3)); env.close(); BEAST_EXPECT(checksOnAccount(env, alice).size() == 0); - BEAST_EXPECT(ownerCount(env, alice) == signersCount + 0); + BEAST_EXPECT(ownerCount(env, alice) == 1); } } diff --git a/src/test/app/MultiSign_test.cpp b/src/test/app/MultiSign_test.cpp index 73b10af339..126413d8d0 100644 --- a/src/test/app/MultiSign_test.cpp +++ b/src/test/app/MultiSign_test.cpp @@ -54,37 +54,31 @@ public: 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. auto const fee = env.current()->fees().base; - auto const smallSignersReserve = reserve1 ? XRP(250) : XRP(350); - env.fund(smallSignersReserve - drops(1), alice); + env.fund(XRP(250) - drops(1), alice); env.close(); env.require(owners(alice, 0)); { // Attach a signer list to alice. Should fail. - Json::Value smallSigners = signers(alice, 1, {{bogie, 1}}); - env(smallSigners, ter(tecINSUFFICIENT_RESERVE)); + Json::Value signersList = signers(alice, 1, {{bogie, 1}}); + env(signersList, ter(tecINSUFFICIENT_RESERVE)); env.close(); env.require(owners(alice, 0)); // Fund alice enough to set the signer list, then attach signers. env(pay(env.master, alice, fee + drops(1))); env.close(); - env(smallSigners); + env(signersList); env.close(); - env.require(owners(alice, reserve1 ? 1 : 3)); + env.require(owners(alice, 1)); } { // 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))); + env(pay(env.master, alice, fee - drops(1))); // Replace with the biggest possible signer list. Should fail. Json::Value bigSigners = signers( @@ -100,14 +94,14 @@ public: {spook, 1}}); env(bigSigners, ter(tecINSUFFICIENT_RESERVE)); env.close(); - env.require(owners(alice, reserve1 ? 1 : 3)); + env.require(owners(alice, 1)); // 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, reserve1 ? 1 : 10)); + env.require(owners(alice, 1)); } // Remove alice's signer list and get the owner count back. env(signers(alice, jtx::none)); @@ -170,14 +164,12 @@ public: ter(temBAD_QUORUM)); // clang-format off - // Make a signer list that's too big. Should fail. (Even with - // ExpandedSignerList) + // Make a signer list that's too big. Should fail. Account const spare("spare", KeyType::secp256k1); env(signers( alice, 1, - features[featureExpandedSignerList] - ? std::vector{{bogie, 1}, {demon, 1}, {ghost, 1}, + std::vector{{bogie, 1}, {demon, 1}, {ghost, 1}, {haunt, 1}, {jinni, 1}, {phase, 1}, {shade, 1}, {spook, 1}, {spare, 1}, {acc10, 1}, {acc11, 1}, {acc12, 1}, @@ -187,10 +179,7 @@ public: {acc22, 1}, {acc23, 1}, {acc24, 1}, {acc25, 1}, {acc26, 1}, {acc27, 1}, {acc28, 1}, {acc29, 1}, {acc30, 1}, - {acc31, 1}, {acc32, 1}, {acc33, 1}} - : std::vector{{bogie, 1}, {demon, 1}, {ghost, 1}, - {haunt, 1}, {jinni, 1}, {phase, 1}, - {shade, 1}, {spook, 1}, {spare, 1}}), + {acc31, 1}, {acc32, 1}, {acc33, 1}}), ter(temMALFORMED)); // clang-format on env.close(); @@ -211,7 +200,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, features[featureMultiSignReserve] ? 1 : 4)); + env.require(owners(alice, 1)); // This should work. auto const baseFee = env.current()->fees().base; @@ -288,7 +277,7 @@ public: {shade, 1}, {spook, 1}})); env.close(); - env.require(owners(alice, features[featureMultiSignReserve] ? 1 : 10)); + env.require(owners(alice, 1)); // This should work. auto const baseFee = env.current()->fees().base; @@ -343,7 +332,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, features[featureMultiSignReserve] ? 1 : 4)); + env.require(owners(alice, 1)); msig phantoms{bogie, demon}; std::reverse(phantoms.signers.begin(), phantoms.signers.end()); @@ -382,7 +371,7 @@ public: // Attach signers to alice env(signers(alice, 4, {{becky, 3}, {cheri, 4}}), sig(alice)); env.close(); - env.require(owners(alice, features[featureMultiSignReserve] ? 1 : 4)); + env.require(owners(alice, 1)); // Attempt a multisigned transaction that meets the quorum. auto const baseFee = env.current()->fees().base; @@ -751,7 +740,7 @@ public: env(signers(alice, 1, {{becky, 1}, {cheri, 1}, {daria, 1}, {jinni, 1}}), sig(alie)); env.close(); - env.require(owners(alice, features[featureMultiSignReserve] ? 1 : 6)); + env.require(owners(alice, 1)); // Each type of signer should succeed individually. auto const baseFee = env.current()->fees().base; @@ -798,7 +787,7 @@ public: {jinni, 0xFFFF}}), sig(alie)); env.close(); - env.require(owners(alice, features[featureMultiSignReserve] ? 1 : 6)); + env.require(owners(alice, 1)); aliceSeq = env.seq(alice); env(noop(alice), @@ -829,7 +818,7 @@ public: {spook, 0xFFFF}}), sig(alie)); env.close(); - env.require(owners(alice, features[featureMultiSignReserve] ? 1 : 10)); + env.require(owners(alice, 1)); aliceSeq = env.seq(alice); env(noop(alice), @@ -1005,8 +994,7 @@ public: // Attach signers to alice. env(signers(alice, 2, {{becky, 1}, {bogie, 1}}), sig(alie)); env.close(); - int const signerListOwners{features[featureMultiSignReserve] ? 1 : 4}; - env.require(owners(alice, signerListOwners + 0)); + env.require(owners(alice, 1)); // Multisign a ttPAYMENT. auto const baseFee = env.current()->fees().base; @@ -1036,7 +1024,7 @@ public: fee(3 * baseFee), require(lines("alice", 1))); env.close(); - env.require(owners(alice, signerListOwners + 1)); + env.require(owners(alice, 2)); // Multisign a ttOFFER_CREATE transaction. env(pay(gw, alice, USD(50))); @@ -1049,7 +1037,7 @@ public: msig(becky, bogie), fee(3 * baseFee)); env.close(); - env.require(owners(alice, signerListOwners + 2)); + env.require(owners(alice, 3)); // Now multisign a ttOFFER_CANCEL canceling the offer we just created. { @@ -1060,7 +1048,7 @@ public: fee(3 * baseFee)); env.close(); BEAST_EXPECT(env.seq(alice) == aliceSeq + 1); - env.require(owners(alice, signerListOwners + 1)); + env.require(owners(alice, 2)); } // Multisign a ttSIGNER_LIST_SET. @@ -1068,7 +1056,7 @@ public: msig(becky, bogie), fee(3 * baseFee)); env.close(); - env.require(owners(alice, features[featureMultiSignReserve] ? 2 : 6)); + env.require(owners(alice, 2)); } void @@ -1178,56 +1166,44 @@ public: "fails local checks: Invalid Signers array size."); } { - // Multisign 9 (!ExpandedSignerList) | 33 (ExpandedSignerList) times - // should fail. JTx tx = env.jt( noop(alice), fee(2 * baseFee), - features[featureExpandedSignerList] ? msig( - bogie, - bogie, - bogie, - bogie, - bogie, - bogie, - bogie, - bogie, - bogie, - bogie, - bogie, - bogie, - bogie, - bogie, - bogie, - bogie, - bogie, - bogie, - bogie, - bogie, - bogie, - bogie, - bogie, - bogie, - bogie, - bogie, - bogie, - bogie, - bogie, - bogie, - bogie, - bogie, - bogie) - : msig( - bogie, - bogie, - bogie, - bogie, - bogie, - bogie, - bogie, - bogie, - bogie)); + msig( + bogie, + bogie, + bogie, + bogie, + bogie, + bogie, + bogie, + bogie, + bogie, + bogie, + bogie, + bogie, + bogie, + bogie, + bogie, + bogie, + bogie, + bogie, + bogie, + bogie, + bogie, + bogie, + bogie, + bogie, + bogie, + bogie, + bogie, + bogie, + bogie, + bogie, + bogie, + bogie, + bogie)); STTx local = *(tx.stx); auto const info = submitSTTx(local); BEAST_EXPECT( @@ -1446,101 +1422,6 @@ public: .asString() == "tesSUCCESS"); } - void - testAmendmentTransition() - { - 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, testable_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 testSignersWithTickets(FeatureBitset features) { @@ -1585,9 +1466,6 @@ public: void testSignersWithTags(FeatureBitset features) { - if (!features[featureExpandedSignerList]) - return; - testcase("Signers With Tags"); using namespace jtx; @@ -1609,7 +1487,7 @@ public: // Attach phantom signers to alice and use them for a transaction. env(signers(alice, 1, {{bogie, 1, bogie_tag}, {demon, 1, demon_tag}})); env.close(); - env.require(owners(alice, features[featureMultiSignReserve] ? 1 : 4)); + env.require(owners(alice, 1)); // This should work. auto const baseFee = env.current()->fees().base; @@ -1744,12 +1622,6 @@ public: using namespace jtx; auto const all = testable_amendments(); - // The reserve required on a signer list changes based on - // featureMultiSignReserve. Limits on the number of signers - // changes based on featureExpandedSignerList. Test both with and - // without. - testAll(all - featureMultiSignReserve - featureExpandedSignerList); - testAll(all - featureExpandedSignerList); testAll(all); testSignerListSetFlags(all - fixInvalidTxFlags); @@ -1757,8 +1629,6 @@ public: testSignerListObject(all - fixIncludeKeyletFields); testSignerListObject(all); - - testAmendmentTransition(); } }; diff --git a/src/test/app/Oracle_test.cpp b/src/test/app/Oracle_test.cpp index 91f2942cd2..42f2455f31 100644 --- a/src/test/app/Oracle_test.cpp +++ b/src/test/app/Oracle_test.cpp @@ -727,13 +727,13 @@ private: } void - testMultisig(FeatureBitset features) + testMultisig() { testcase("Multisig"); using namespace jtx; Oracle::setFee(100'000); - Env env(*this, features); + Env env(*this); auto const baseFee = static_cast(env.current()->fees().base.drops()); @@ -753,9 +753,8 @@ private: // Attach signers to alice. env(signers(alice, 2, {{becky, 1}, {bogie, 1}, {ed, 2}}), sig(alie)); env.close(); - // if multiSignReserve disabled then its 2 + 1 per signer - int const signerListOwners{features[featureMultiSignReserve] ? 1 : 5}; - env.require(owners(alice, signerListOwners)); + + env.require(owners(alice, 1)); // Create // Force close (true) and time advancement because the close time @@ -860,11 +859,7 @@ public: testDelete(); testUpdate(); testAmendment(); - for (auto const& features : - {all, - all - featureMultiSignReserve - featureExpandedSignerList, - all - featureExpandedSignerList}) - testMultisig(features); + testMultisig(); } }; diff --git a/src/test/jtx/Env_test.cpp b/src/test/jtx/Env_test.cpp index 7ec453c9ff..5e4f534b6a 100644 --- a/src/test/jtx/Env_test.cpp +++ b/src/test/jtx/Env_test.cpp @@ -798,17 +798,16 @@ public: { // a Env FeatureBitset has *only* those features - Env env{*this, FeatureBitset(featureMultiSignReserve, featureFlow)}; + Env env{*this, FeatureBitset{featureDynamicMPT | featureFlow}}; BEAST_EXPECT(env.app().config().features.size() == 2); foreachFeature(supported, [&](uint256 const& f) { - bool const has = - (f == featureMultiSignReserve || f == featureFlow); + bool const has = (f == featureDynamicMPT || f == featureFlow); this->BEAST_EXPECT(has == hasFeature(env, f)); }); } auto const missingSomeFeatures = - testable_amendments() - featureMultiSignReserve - featureFlow; + testable_amendments() - featureDynamicMPT - featureFlow; BEAST_EXPECT(missingSomeFeatures.count() == (supported.count() - 2)); { // a Env supported_features_except is missing *only* those features @@ -816,8 +815,7 @@ public: BEAST_EXPECT( env.app().config().features.size() == (supported.count() - 2)); foreachFeature(supported, [&](uint256 const& f) { - bool hasnot = - (f == featureMultiSignReserve || f == featureFlow); + bool hasnot = (f == featureDynamicMPT || f == featureFlow); this->BEAST_EXPECT(hasnot != hasFeature(env, f)); }); } @@ -829,8 +827,8 @@ public: // the two supported ones Env env{ *this, - FeatureBitset( - featureMultiSignReserve, featureFlow, *neverSupportedFeat)}; + FeatureBitset{ + featureDynamicMPT, featureFlow, *neverSupportedFeat}}; // this app will have just 2 supported amendments and // one additional never supported feature flag @@ -838,7 +836,7 @@ public: BEAST_EXPECT(hasFeature(env, *neverSupportedFeat)); foreachFeature(supported, [&](uint256 const& f) { - bool has = (f == featureMultiSignReserve || f == featureFlow); + bool has = (f == featureDynamicMPT || f == featureFlow); this->BEAST_EXPECT(has == hasFeature(env, f)); }); } @@ -858,8 +856,7 @@ public: (supported.count() - 2 + 1)); BEAST_EXPECT(hasFeature(env, *neverSupportedFeat)); foreachFeature(supported, [&](uint256 const& f) { - bool hasnot = - (f == featureMultiSignReserve || f == featureFlow); + bool hasnot = (f == featureDynamicMPT || f == featureFlow); this->BEAST_EXPECT(hasnot != hasFeature(env, f)); }); } diff --git a/src/test/protocol/STTx_test.cpp b/src/test/protocol/STTx_test.cpp index 572219c357..8a434486ea 100644 --- a/src/test/protocol/STTx_test.cpp +++ b/src/test/protocol/STTx_test.cpp @@ -1612,7 +1612,7 @@ public: // proper lifetime. std::unordered_set> const presets; Rules const defaultRules{presets}; - BEAST_EXPECT(!defaultRules.enabled(featureExpandedSignerList)); + BEAST_EXPECT(!defaultRules.enabled(featureAMM)); unexpected( !j.checkSign(STTx::RequireFullyCanonicalSig::yes, defaultRules), diff --git a/src/test/rpc/Feature_test.cpp b/src/test/rpc/Feature_test.cpp index 13d5fd3969..250c634007 100644 --- a/src/test/rpc/Feature_test.cpp +++ b/src/test/rpc/Feature_test.cpp @@ -183,16 +183,16 @@ class Feature_test : public beast::unit_test::suite using namespace test::jtx; Env env{*this}; - auto jrr = env.rpc("feature", "MultiSignReserve")[jss::result]; + auto jrr = env.rpc("feature", "RequireFullyCanonicalSig")[jss::result]; BEAST_EXPECTS(jrr[jss::status] == jss::success, "status"); jrr.removeMember(jss::status); BEAST_EXPECT(jrr.size() == 1); BEAST_EXPECT( - jrr.isMember("586480873651E106F1D6339B0C4A8945BA705A777F3F4524626FF" - "1FC07EFE41D")); + jrr.isMember("00C1FC4A53E60AB02C864641002B3172F38677E29C26C54066851" + "79B37E1EDAC")); auto feature = *(jrr.begin()); - BEAST_EXPECTS(feature[jss::name] == "MultiSignReserve", "name"); + BEAST_EXPECTS(feature[jss::name] == "RequireFullyCanonicalSig", "name"); BEAST_EXPECTS(!feature[jss::enabled].asBool(), "enabled"); BEAST_EXPECTS( feature[jss::vetoed].isBool() && !feature[jss::vetoed].asBool(), @@ -200,7 +200,7 @@ class Feature_test : public beast::unit_test::suite BEAST_EXPECTS(feature[jss::supported].asBool(), "supported"); // feature names are case-sensitive - expect error here - jrr = env.rpc("feature", "multiSignReserve")[jss::result]; + jrr = env.rpc("feature", "requireFullyCanonicalSig")[jss::result]; BEAST_EXPECT(jrr[jss::error] == "badFeature"); BEAST_EXPECT(jrr[jss::error_message] == "Feature unknown or invalid."); } @@ -476,8 +476,8 @@ class Feature_test : public beast::unit_test::suite testcase("Veto"); using namespace test::jtx; - Env env{*this, FeatureBitset(featureMultiSignReserve)}; - constexpr char const* featureName = "MultiSignReserve"; + Env env{*this, FeatureBitset{featureRequireFullyCanonicalSig}}; + constexpr char const* featureName = "RequireFullyCanonicalSig"; auto jrr = env.rpc("feature", featureName)[jss::result]; if (!BEAST_EXPECTS(jrr[jss::status] == jss::success, "status")) diff --git a/src/xrpld/app/tx/detail/SetSignerList.cpp b/src/xrpld/app/tx/detail/SetSignerList.cpp index a5409da700..449e0ff90c 100644 --- a/src/xrpld/app/tx/detail/SetSignerList.cpp +++ b/src/xrpld/app/tx/detail/SetSignerList.cpp @@ -142,10 +142,6 @@ SetSignerList::preCompute() // The return type is signed so it is compatible with the 3rd argument // of adjustOwnerCount() (which must be signed). -// -// NOTE: This way of computing the OwnerCount associated with a SignerList -// is valid until the featureMultiSignReserve amendment passes. Once it -// passes then just 1 OwnerCount is associated with a SignerList. static int signerCountBasedOwnerCountDelta(std::size_t entryCount, Rules const& rules) { @@ -162,13 +158,13 @@ signerCountBasedOwnerCountDelta(std::size_t entryCount, Rules const& rules) // units. A SignerList with 8 entries would cost 10 OwnerCount units. // // The static_cast should always be safe since entryCount should always - // be in the range from 1 to 8 (or 32 if ExpandedSignerList is enabled). + // be in the range from 1 to 32. // We've got a lot of room to grow. XRPL_ASSERT( entryCount >= STTx::minMultiSigners, "ripple::signerCountBasedOwnerCountDelta : minimum signers"); XRPL_ASSERT( - entryCount <= STTx::maxMultiSigners(&rules), + entryCount <= STTx::maxMultiSigners, "ripple::signerCountBasedOwnerCountDelta : maximum signers"); return 2 + static_cast(entryCount); } @@ -250,8 +246,8 @@ SetSignerList::validateQuorumAndSignerEntries( // Reject if there are too many or too few entries in the list. { std::size_t const signerCount = signers.size(); - if ((signerCount < STTx::minMultiSigners) || - (signerCount > STTx::maxMultiSigners(&rules))) + if (signerCount < STTx::minMultiSigners || + signerCount > STTx::maxMultiSigners) { JLOG(j.trace()) << "Too many or too few signers in signer list."; return temMALFORMED; @@ -269,9 +265,6 @@ SetSignerList::validateQuorumAndSignerEntries( return temBAD_SIGNER; } - // Is the ExpandedSignerList amendment active? - bool const expandedSignerList = rules.enabled(featureExpandedSignerList); - // Make sure no signers reference this account. Also make sure the // quorum can be reached. std::uint64_t allSignersWeight(0); @@ -291,15 +284,6 @@ SetSignerList::validateQuorumAndSignerEntries( JLOG(j.trace()) << "A signer may not self reference account."; return temBAD_SIGNER; } - - if (signer.tag && !expandedSignerList) - { - JLOG(j.trace()) << "Malformed transaction: sfWalletLocator " - "specified in SignerEntry " - << "but featureExpandedSignerList is not enabled."; - return temMALFORMED; - } - // Don't verify that the signer accounts exist. Non-existent accounts // may be phantom accounts (which are permitted). } @@ -337,15 +321,8 @@ SetSignerList::replaceSignerList() // Compute new reserve. Verify the account has funds to meet the reserve. std::uint32_t const oldOwnerCount{(*sle)[sfOwnerCount]}; - // The required reserve changes based on featureMultiSignReserve... - int addedOwnerCount{1}; + constexpr int addedOwnerCount = 1; std::uint32_t flags{lsfOneOwnerCount}; - if (!ctx_.view().rules().enabled(featureMultiSignReserve)) - { - addedOwnerCount = signerCountBasedOwnerCountDelta( - signers_.size(), ctx_.view().rules()); - flags = 0; - } XRPAmount const newReserve{ view().fees().accountReserve(oldOwnerCount + addedOwnerCount)}; @@ -415,9 +392,6 @@ SetSignerList::writeSignersToSLE( if (flags) // Only set flags if they are non-default (default is zero). ledgerEntry->setFieldU32(sfFlags, flags); - bool const expandedSignerList = - ctx_.view().rules().enabled(featureExpandedSignerList); - // Create the SignerListArray one SignerEntry at a time. STArray toLedger(signers_.size()); for (auto const& entry : signers_) @@ -429,8 +403,8 @@ SetSignerList::writeSignersToSLE( obj[sfSignerWeight] = entry.weight; // This is a defensive check to make absolutely sure we will never write - // a tag into the ledger while featureExpandedSignerList is not enabled - if (expandedSignerList && entry.tag) + // a tag into the ledger. + if (entry.tag) obj.setFieldH256(sfWalletLocator, *(entry.tag)); } diff --git a/src/xrpld/app/tx/detail/SignerEntries.cpp b/src/xrpld/app/tx/detail/SignerEntries.cpp index 3d551eb30a..f9bc939190 100644 --- a/src/xrpld/app/tx/detail/SignerEntries.cpp +++ b/src/xrpld/app/tx/detail/SignerEntries.cpp @@ -25,7 +25,7 @@ SignerEntries::deserialize( } std::vector accountVec; - accountVec.reserve(STTx::maxMultiSigners()); + accountVec.reserve(STTx::maxMultiSigners); STArray const& sEntries(obj.getFieldArray(sfSignerEntries)); for (STObject const& sEntry : sEntries) diff --git a/src/xrpld/rpc/detail/TransactionSign.cpp b/src/xrpld/rpc/detail/TransactionSign.cpp index 2308280597..5e9e2d9cc4 100644 --- a/src/xrpld/rpc/detail/TransactionSign.cpp +++ b/src/xrpld/rpc/detail/TransactionSign.cpp @@ -822,7 +822,7 @@ getTxFee(Application const& app, Config const& config, Json::Value tx) if (!tx[jss::Signers].isArray()) return config.FEES.reference_fee; - if (tx[jss::Signers].size() > STTx::maxMultiSigners(&ledger->rules())) + if (tx[jss::Signers].size() > STTx::maxMultiSigners) return config.FEES.reference_fee; // check multi-signed signers