diff --git a/.github/workflows/reusable-build-test-config.yml b/.github/workflows/reusable-build-test-config.yml index a57d15275b..4d20bfbf35 100644 --- a/.github/workflows/reusable-build-test-config.yml +++ b/.github/workflows/reusable-build-test-config.yml @@ -69,9 +69,9 @@ jobs: ENABLED_VOIDSTAR: ${{ contains(inputs.cmake_args, '-Dvoidstar=ON') }} ENABLED_COVERAGE: ${{ contains(inputs.cmake_args, '-Dcoverage=ON') }} steps: - - name: Cleanup workspace - if: ${{ runner.os == 'macOS' }} - uses: XRPLF/actions/.github/actions/cleanup-workspace@3f044c7478548e3c32ff68980eeb36ece02b364e + - name: Cleanup workspace (macOS and Windows) + if: ${{ runner.os == 'macOS' || runner.os == 'Windows' }} + uses: XRPLF/actions/.github/actions/cleanup-workspace@01b244d2718865d427b499822fbd3f15e7197fcc - name: Checkout repository uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0 diff --git a/.github/workflows/upload-conan-deps.yml b/.github/workflows/upload-conan-deps.yml index 853b0f0dac..14d5c64ec2 100644 --- a/.github/workflows/upload-conan-deps.yml +++ b/.github/workflows/upload-conan-deps.yml @@ -62,9 +62,9 @@ jobs: runs-on: ${{ matrix.architecture.runner }} container: ${{ contains(matrix.architecture.platform, 'linux') && format('ghcr.io/xrplf/ci/{0}-{1}:{2}-{3}-sha-{4}', matrix.os.distro_name, matrix.os.distro_version, matrix.os.compiler_name, matrix.os.compiler_version, matrix.os.image_sha) || null }} steps: - - name: Cleanup workspace - if: ${{ runner.os == 'macOS' }} - uses: XRPLF/actions/.github/actions/cleanup-workspace@3f044c7478548e3c32ff68980eeb36ece02b364e + - name: Cleanup workspace (macOS and Windows) + if: ${{ runner.os == 'macOS' || runner.os == 'Windows' }} + uses: XRPLF/actions/.github/actions/cleanup-workspace@01b244d2718865d427b499822fbd3f15e7197fcc - name: Checkout repository uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0 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/basics/BasicConfig.cpp b/src/libxrpl/basics/BasicConfig.cpp index 280c660794..63a001bcc3 100644 --- a/src/libxrpl/basics/BasicConfig.cpp +++ b/src/libxrpl/basics/BasicConfig.cpp @@ -30,7 +30,7 @@ Section::append(std::vector const& lines) // '=' static boost::regex const re1( "^" // start of line - "(?:\\s*)" // whitespace (optonal) + "(?:\\s*)" // whitespace (optional) "([a-zA-Z][_a-zA-Z0-9]*)" // "(?:\\s*)" // whitespace (optional) "(?:=)" // '=' diff --git a/src/libxrpl/protocol/BuildInfo.cpp b/src/libxrpl/protocol/BuildInfo.cpp index af21ce5985..7690ae586b 100644 --- a/src/libxrpl/protocol/BuildInfo.cpp +++ b/src/libxrpl/protocol/BuildInfo.cpp @@ -17,7 +17,7 @@ namespace BuildInfo { // and follow the format described at http://semver.org/ //------------------------------------------------------------------------------ // clang-format off -char const* const versionString = "3.0.0-b1" +char const* const versionString = "3.1.0-b0" // clang-format on #if defined(DEBUG) || defined(SANITIZER) 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/NFToken_test.cpp b/src/test/app/NFToken_test.cpp index 238042d388..a11446df71 100644 --- a/src/test/app/NFToken_test.cpp +++ b/src/test/app/NFToken_test.cpp @@ -6808,7 +6808,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite mintAndCreateSellOffer(env, alice, XRP(0)); // Bob can accept the offer because the new NFT is stored in - // an existing NFTokenPage so no new reserve is requried + // an existing NFTokenPage so no new reserve is required env(token::acceptSellOffer(bob, sellOfferIndex)); env.close(); } diff --git a/src/test/app/NetworkOPs_test.cpp b/src/test/app/NetworkOPs_test.cpp index 166c3140d4..d9afe6d628 100644 --- a/src/test/app/NetworkOPs_test.cpp +++ b/src/test/app/NetworkOPs_test.cpp @@ -21,7 +21,7 @@ public: void testAllBadHeldTransactions() { - // All trasactions are already marked as SF_BAD, and we should be able + // All transactions are already marked as SF_BAD, and we should be able // to handle the case properly without an assertion failure testcase("No valid transactions in batch"); 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/app/PayChan_test.cpp b/src/test/app/PayChan_test.cpp index 893d71bfa8..bee3167b29 100644 --- a/src/test/app/PayChan_test.cpp +++ b/src/test/app/PayChan_test.cpp @@ -1889,7 +1889,6 @@ struct PayChan_test : public beast::unit_test::suite BEAST_EXPECT(channelAmount(*env.current(), chan) == XRP(1000)); rmAccount(env, alice, carol, tecHAS_OBLIGATIONS); - // can only remove bob if the channel isn't in their owner direcotry rmAccount(env, bob, carol, TER(tecHAS_OBLIGATIONS)); auto const feeDrops = env.current()->fees().base; @@ -1904,7 +1903,6 @@ struct PayChan_test : public beast::unit_test::suite auto authAmt = reqBal + XRP(100); assert(reqBal <= chanAmt); - // claim should fail if the dst was removed env(claim(alice, chan, reqBal, authAmt)); env.close(); BEAST_EXPECT(channelBalance(*env.current(), chan) == reqBal); @@ -1912,7 +1910,6 @@ struct PayChan_test : public beast::unit_test::suite BEAST_EXPECT(env.balance(bob) == preBob + delta); chanBal = reqBal; - // fund should fail if the dst was removed auto const preAlice = env.balance(alice); env(fund(alice, chan, XRP(1000))); env.close(); @@ -1925,7 +1922,7 @@ struct PayChan_test : public beast::unit_test::suite // Owner closes, will close after settleDelay env(claim(alice, chan), txflags(tfClose)); env.close(); - // settle delay hasn't ellapsed. Channels should exist. + // settle delay hasn't elapsed. Channels should exist. BEAST_EXPECT(channelExists(*env.current(), chan)); auto const closeTime = env.current()->info().parentCloseTime; auto const minExpiration = closeTime + settleDelay; diff --git a/src/test/csf/Peer.h b/src/test/csf/Peer.h index 3a76e7b841..5f6f006b0f 100644 --- a/src/test/csf/Peer.h +++ b/src/test/csf/Peer.h @@ -404,7 +404,7 @@ struct Peer { minDuration = std::min(minDuration, link.data.delay); - // Send a messsage to neighbors to find the ledger + // Send a message to neighbors to find the ledger net.send( this, link.target, [to = link.target, from = this, ledgerID]() { if (auto it = to->ledgers.find(ledgerID); diff --git a/src/test/csf/collectors.h b/src/test/csf/collectors.h index c361701b4e..53494dad44 100644 --- a/src/test/csf/collectors.h +++ b/src/test/csf/collectors.h @@ -28,7 +28,7 @@ namespace csf { /** Group of collectors. Presents a group of collectors as a single collector which process an event - by calling each collector sequentially. This is analagous to CollectorRefs + by calling each collector sequentially. This is analogous to CollectorRefs in CollectorRef.h, but does *not* erase the type information of the combined collectors. */ diff --git a/src/test/csf/events.h b/src/test/csf/events.h index bf01de4eb2..b25dc4b339 100644 --- a/src/test/csf/events.h +++ b/src/test/csf/events.h @@ -13,7 +13,7 @@ namespace test { namespace csf { // Events are emitted by peers at a variety of points during the simulation. -// Each event is emitted by a particlar peer at a particular time. Collectors +// Each event is emitted by a particular peer at a particular time. Collectors // process these events, perhaps calculating statistics or storing events to // a log for post-processing. // diff --git a/src/test/csf/ledgers.h b/src/test/csf/ledgers.h index 26b416a2fb..80e431c1e2 100644 --- a/src/test/csf/ledgers.h +++ b/src/test/csf/ledgers.h @@ -35,7 +35,7 @@ namespace csf { Ledgers are immutable value types. All ledgers with the same sequence number, transactions, close time, etc. will have the same ledger ID. The - LedgerOracle class below manges ID assignments for a simulation and is the + LedgerOracle class below manages ID assignments for a simulation and is the only way to close and create a new ledger. Since the parent ledger ID is part of type, this also means ledgers with distinct histories will have distinct ids, even if they have the same set of transactions, sequence 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/nodestore/TestBase.h b/src/test/nodestore/TestBase.h index c1401abc33..0675ad85d4 100644 --- a/src/test/nodestore/TestBase.h +++ b/src/test/nodestore/TestBase.h @@ -81,7 +81,7 @@ public: case 3: return hotUNKNOWN; } - // will never happen, but make static analysys tool happy. + // will never happen, but make static analysis tool happy. return hotUNKNOWN; }(); diff --git a/src/test/nodestore/import_test.cpp b/src/test/nodestore/import_test.cpp index 69ef50d453..30aaaa2ea9 100644 --- a/src/test/nodestore/import_test.cpp +++ b/src/test/nodestore/import_test.cpp @@ -235,7 +235,7 @@ parse_args(std::string const& s) // '=' static boost::regex const re1( "^" // start of line - "(?:\\s*)" // whitespace (optonal) + "(?:\\s*)" // whitespace (optional) "([a-zA-Z][_a-zA-Z0-9]*)" // "(?:\\s*)" // whitespace (optional) "(?:=)" // '=' diff --git a/src/test/overlay/tx_reduce_relay_test.cpp b/src/test/overlay/tx_reduce_relay_test.cpp index 3eafa08713..1ca9e7aac6 100644 --- a/src/test/overlay/tx_reduce_relay_test.cpp +++ b/src/test/overlay/tx_reduce_relay_test.cpp @@ -245,7 +245,7 @@ private: // (20+0.25*(60-20)-5=25), queue the rest, skip counts towards relayed // (60-25-5=30) testRelay("skip", true, 60, 0, 20, 25, 25, 30, skip); - // relay to minPeers + disabled + 25% of (nPeers - minPeers - disalbed) + // relay to minPeers + disabled + 25% of (nPeers - minPeers - disabled) // (20+10+0.25*(70-20-10)=40), queue the rest (30) testRelay("disabled", true, 70, 10, 20, 25, 40, 30); // relay to minPeers + disabled-not-in-skip + 25% of (nPeers - minPeers 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/test/rpc/NoRippleCheck_test.cpp b/src/test/rpc/NoRippleCheck_test.cpp index 94c85a59ef..81ec097181 100644 --- a/src/test/rpc/NoRippleCheck_test.cpp +++ b/src/test/rpc/NoRippleCheck_test.cpp @@ -252,7 +252,7 @@ class NoRippleCheckLimits_test : public beast::unit_test::suite auto checkBalance = [&env]() { // this is endpoint drop prevention. Non admin ports will drop // requests if they are coming too fast, so we manipulate the - // resource manager here to reset the enpoint balance (for + // resource manager here to reset the endpoint balance (for // localhost) if we get too close to the drop limit. It would // be better if we could add this functionality to Env somehow // or otherwise disable endpoint charging for certain test 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