ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2025-11-28 23:15:52 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor: Retire MultiSignReserve and ExpandedSignerList amendments (#5981)

Browse Source 
Amendments activated for more than 2 years can be retired. This change retires the MultiSignReserve and ExpandedSignerList amendments.
This commit is contained in:
Jingchen
2025-11-13 11:42:45 +00:00
committed by GitHub
parent 8580a5795a
commit 508937f3d1
13 changed files with 105 additions and 306 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
include/xrpl/protocol/STTx.h
View File

@@ -31,17 +31,8 @@ class STTx final : public STObject, public CountedObject<STTx>
TxType tx_type_; TxType tx_type_;
public: public:
static std::size_t const minMultiSigners = 1; static constexpr std::size_t minMultiSigners = 1;
static constexpr std::size_t maxMultiSigners = 32;
// 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;
}
STTx() = delete; STTx() = delete;
STTx(STTx const& other) = default; STTx(STTx const& other) = default;

4
include/xrpl/protocol/detail/features.macro
View File

@@ -65,13 +65,11 @@ XRPL_FIX (UniversalNumber, Supported::yes, VoteBehavior::DefaultNo
XRPL_FEATURE(XRPFees, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(XRPFees, Supported::yes, VoteBehavior::DefaultNo)
XRPL_FEATURE(DisallowIncoming, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(DisallowIncoming, Supported::yes, VoteBehavior::DefaultNo)
XRPL_FIX (RemoveNFTokenAutoTrustLine, Supported::yes, VoteBehavior::DefaultYes) XRPL_FIX (RemoveNFTokenAutoTrustLine, Supported::yes, VoteBehavior::DefaultYes)
XRPL_FEATURE(ExpandedSignerList, Supported::yes, VoteBehavior::DefaultNo)
XRPL_FEATURE(FlowSortStrands, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(FlowSortStrands, Supported::yes, VoteBehavior::DefaultYes)
XRPL_FEATURE(TicketBatch, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(TicketBatch, Supported::yes, VoteBehavior::DefaultYes)
XRPL_FEATURE(NegativeUNL, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(NegativeUNL, Supported::yes, VoteBehavior::DefaultYes)
XRPL_FEATURE(RequireFullyCanonicalSig, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(RequireFullyCanonicalSig, Supported::yes, VoteBehavior::DefaultYes)
XRPL_FEATURE(DeletableAccounts, 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(Checks, Supported::yes, VoteBehavior::DefaultYes)
XRPL_FEATURE(Flow, 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(DepositPreauth)
XRPL_RETIRE_FEATURE(Escrow) XRPL_RETIRE_FEATURE(Escrow)
XRPL_RETIRE_FEATURE(EnforceInvariants) XRPL_RETIRE_FEATURE(EnforceInvariants)
XRPL_RETIRE_FEATURE(ExpandedSignerList)
XRPL_RETIRE_FEATURE(FeeEscalation) XRPL_RETIRE_FEATURE(FeeEscalation)
XRPL_RETIRE_FEATURE(FlowCross) XRPL_RETIRE_FEATURE(FlowCross)
XRPL_RETIRE_FEATURE(HardenedValidations) XRPL_RETIRE_FEATURE(HardenedValidations)
XRPL_RETIRE_FEATURE(ImmediateOfferKilled) XRPL_RETIRE_FEATURE(ImmediateOfferKilled)
XRPL_RETIRE_FEATURE(MultiSign) XRPL_RETIRE_FEATURE(MultiSign)
XRPL_RETIRE_FEATURE(MultiSignReserve)
XRPL_RETIRE_FEATURE(NonFungibleTokensV1_1) XRPL_RETIRE_FEATURE(NonFungibleTokensV1_1)
XRPL_RETIRE_FEATURE(PayChan) XRPL_RETIRE_FEATURE(PayChan)
XRPL_RETIRE_FEATURE(SortedDirectories) XRPL_RETIRE_FEATURE(SortedDirectories)

2
src/libxrpl/protocol/STTx.cpp
View File

@@ -476,7 +476,7 @@ multiSignHelper(
// There are well known bounds that the number of signers must be within. // There are well known bounds that the number of signers must be within.
if (signers.size() < STTx::minMultiSigners || if (signers.size() < STTx::minMultiSigners ||
signers.size() > STTx::maxMultiSigners(&rules)) signers.size() > STTx::maxMultiSigners)
return Unexpected("Invalid Signers array size."); return Unexpected("Invalid Signers array size.");
// Signers must be in sorted order by AccountID. // Signers must be in sorted order by AccountID.

14
src/test/app/AMMExtended_test.cpp
View File

@@ -3534,8 +3534,7 @@ private:
// Attach signers to alice. // Attach signers to alice.
env(signers(alice, 2, {{becky, 1}, {bogie, 1}}), sig(alie)); env(signers(alice, 2, {{becky, 1}, {bogie, 1}}), sig(alie));
env.close(); env.close();
int const signerListOwners{features[featureMultiSignReserve] ? 2 : 5}; env.require(owners(alice, 2));
env.require(owners(alice, signerListOwners + 0));
msig const ms{becky, bogie}; msig const ms{becky, bogie};
@@ -3795,20 +3794,13 @@ private:
void void
testMultisign() testMultisign()
{ {
using namespace jtx; testTxMultisign(jtx::testable_amendments());
auto const all = testable_amendments();
testTxMultisign(
all - featureMultiSignReserve - featureExpandedSignerList);
testTxMultisign(all - featureExpandedSignerList);
testTxMultisign(all);
} }
void void
testPayStrand() testPayStrand()
{ {
using namespace jtx; auto const all = jtx::testable_amendments();
auto const all = testable_amendments();
testToStrand(all); testToStrand(all);
testRIPD1373(all); testRIPD1373(all);

38
src/test/app/Check_test.cpp
View File

@@ -963,14 +963,8 @@ class Check_test : public beast::unit_test::suite
} }
// Use a regular key and also multisign to cash a check. // 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.fund(XRP(1000), gw, alice, bob);
env.close(); 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(signers(bob, 2, {{bogie, 1}, {demon, 1}}), sig(bobby));
env.close(); env.close();
// If featureMultiSignReserve is enabled then bob's signer list BEAST_EXPECT(ownerCount(env, bob) == 2);
// has an owner count of 1, otherwise it's 4.
int const signersCount = {
testFeatures[featureMultiSignReserve] ? 1 : 4};
BEAST_EXPECT(ownerCount(env, bob) == signersCount + 1);
// bob uses his regular key to cash a check. // bob uses his regular key to cash a check.
env(check::cash(bob, chkId1, (USD(1))), sig(bobby)); 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, alice).size() == 1);
BEAST_EXPECT(checksOnAccount(env, bob).size() == 1); BEAST_EXPECT(checksOnAccount(env, bob).size() == 1);
BEAST_EXPECT(ownerCount(env, alice) == 2); 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. // bob uses multisigning to cash a check.
XRPAmount const baseFeeDrops{env.current()->fees().base}; 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, alice).size() == 0);
BEAST_EXPECT(checksOnAccount(env, bob).size() == 0); BEAST_EXPECT(checksOnAccount(env, bob).size() == 0);
BEAST_EXPECT(ownerCount(env, alice) == 1); 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"}; Account const zoe{"zoe"};
IOU const USD{gw["USD"]}; 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.fund(XRP(1000), gw, alice, bob, zoe);
env.close(); 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(signers(alice, 2, {{bogie, 1}, {demon, 1}}), sig(alie));
env.close(); 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. // alice uses her regular key to cancel a check.
env(check::cancel(alice, chkIdReg), sig(alie)); env(check::cancel(alice, chkIdReg), sig(alie));
env.close(); env.close();
BEAST_EXPECT(checksOnAccount(env, alice).size() == 3); 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. // alice uses multisigning to cancel a check.
XRPAmount const baseFeeDrops{env.current()->fees().base}; XRPAmount const baseFeeDrops{env.current()->fees().base};
@@ -1747,18 +1727,18 @@ class Check_test : public beast::unit_test::suite
fee(3 * baseFeeDrops)); fee(3 * baseFeeDrops));
env.close(); env.close();
BEAST_EXPECT(checksOnAccount(env, alice).size() == 2); 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. // Creator and destination cancel the remaining unexpired checks.
env(check::cancel(alice, chkId3), sig(alice)); env(check::cancel(alice, chkId3), sig(alice));
env.close(); env.close();
BEAST_EXPECT(checksOnAccount(env, alice).size() == 1); 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(check::cancel(bob, chkIdNotExp3));
env.close(); env.close();
BEAST_EXPECT(checksOnAccount(env, alice).size() == 0); BEAST_EXPECT(checksOnAccount(env, alice).size() == 0);
BEAST_EXPECT(ownerCount(env, alice) == signersCount + 0); BEAST_EXPECT(ownerCount(env, alice) == 1);
} }
} }

246
src/test/app/MultiSign_test.cpp
View File

@@ -54,37 +54,31 @@ public:
Env env{*this, features}; Env env{*this, features};
Account const alice{"alice", KeyType::secp256k1}; 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 // Pay alice enough to meet the initial reserve, but not enough to
// meet the reserve for a SignerListSet. // meet the reserve for a SignerListSet.
auto const fee = env.current()->fees().base; auto const fee = env.current()->fees().base;
auto const smallSignersReserve = reserve1 ? XRP(250) : XRP(350); env.fund(XRP(250) - drops(1), alice);
env.fund(smallSignersReserve - drops(1), alice);
env.close(); env.close();
env.require(owners(alice, 0)); env.require(owners(alice, 0));
{ {
// Attach a signer list to alice. Should fail. // Attach a signer list to alice. Should fail.
Json::Value smallSigners = signers(alice, 1, {{bogie, 1}}); Json::Value signersList = signers(alice, 1, {{bogie, 1}});
env(smallSigners, ter(tecINSUFFICIENT_RESERVE)); env(signersList, ter(tecINSUFFICIENT_RESERVE));
env.close(); env.close();
env.require(owners(alice, 0)); env.require(owners(alice, 0));
// Fund alice enough to set the signer list, then attach signers. // Fund alice enough to set the signer list, then attach signers.
env(pay(env.master, alice, fee + drops(1))); env(pay(env.master, alice, fee + drops(1)));
env.close(); env.close();
env(smallSigners); env(signersList);
env.close(); 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 // Pay alice enough to almost make the reserve for the biggest
// possible list. // possible list.
auto const addReserveBigSigners = reserve1 ? XRP(0) : XRP(350); env(pay(env.master, alice, fee - drops(1)));
env(pay(env.master, alice, addReserveBigSigners + fee - drops(1)));
// Replace with the biggest possible signer list. Should fail. // Replace with the biggest possible signer list. Should fail.
Json::Value bigSigners = signers( Json::Value bigSigners = signers(
@@ -100,14 +94,14 @@ public:
{spook, 1}}); {spook, 1}});
env(bigSigners, ter(tecINSUFFICIENT_RESERVE)); env(bigSigners, ter(tecINSUFFICIENT_RESERVE));
env.close(); env.close();
env.require(owners(alice, reserve1 ? 1 : 3)); env.require(owners(alice, 1));
// Fund alice one more drop (plus the fee) and succeed. // Fund alice one more drop (plus the fee) and succeed.
env(pay(env.master, alice, fee + drops(1))); env(pay(env.master, alice, fee + drops(1)));
env.close(); env.close();
env(bigSigners); env(bigSigners);
env.close(); 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. // Remove alice's signer list and get the owner count back.
env(signers(alice, jtx::none)); env(signers(alice, jtx::none));
@@ -170,14 +164,12 @@ public:
ter(temBAD_QUORUM)); ter(temBAD_QUORUM));
// clang-format off // clang-format off
// Make a signer list that's too big. Should fail. (Even with // Make a signer list that's too big. Should fail.
// ExpandedSignerList)
Account const spare("spare", KeyType::secp256k1); Account const spare("spare", KeyType::secp256k1);
env(signers( env(signers(
alice, alice,
1, 1,
features[featureExpandedSignerList] std::vector<signer>{{bogie, 1}, {demon, 1}, {ghost, 1},
? std::vector<signer>{{bogie, 1}, {demon, 1}, {ghost, 1},
{haunt, 1}, {jinni, 1}, {phase, 1}, {haunt, 1}, {jinni, 1}, {phase, 1},
{shade, 1}, {spook, 1}, {spare, 1}, {shade, 1}, {spook, 1}, {spare, 1},
{acc10, 1}, {acc11, 1}, {acc12, 1}, {acc10, 1}, {acc11, 1}, {acc12, 1},
@@ -187,10 +179,7 @@ public:
{acc22, 1}, {acc23, 1}, {acc24, 1}, {acc22, 1}, {acc23, 1}, {acc24, 1},
{acc25, 1}, {acc26, 1}, {acc27, 1}, {acc25, 1}, {acc26, 1}, {acc27, 1},
{acc28, 1}, {acc29, 1}, {acc30, 1}, {acc28, 1}, {acc29, 1}, {acc30, 1},
{acc31, 1}, {acc32, 1}, {acc33, 1}} {acc31, 1}, {acc32, 1}, {acc33, 1}}),
: std::vector<signer>{{bogie, 1}, {demon, 1}, {ghost, 1},
{haunt, 1}, {jinni, 1}, {phase, 1},
{shade, 1}, {spook, 1}, {spare, 1}}),
ter(temMALFORMED)); ter(temMALFORMED));
// clang-format on // clang-format on
env.close(); env.close();
@@ -211,7 +200,7 @@ public:
// Attach phantom signers to alice and use them for a transaction. // Attach phantom signers to alice and use them for a transaction.
env(signers(alice, 1, {{bogie, 1}, {demon, 1}})); env(signers(alice, 1, {{bogie, 1}, {demon, 1}}));
env.close(); env.close();
env.require(owners(alice, features[featureMultiSignReserve] ? 1 : 4)); env.require(owners(alice, 1));
// This should work. // This should work.
auto const baseFee = env.current()->fees().base; auto const baseFee = env.current()->fees().base;
@@ -288,7 +277,7 @@ public:
{shade, 1}, {shade, 1},
{spook, 1}})); {spook, 1}}));
env.close(); env.close();
env.require(owners(alice, features[featureMultiSignReserve] ? 1 : 10)); env.require(owners(alice, 1));
// This should work. // This should work.
auto const baseFee = env.current()->fees().base; auto const baseFee = env.current()->fees().base;
@@ -343,7 +332,7 @@ public:
// Make sure the transaction fails if they are not. // Make sure the transaction fails if they are not.
env(signers(alice, 1, {{bogie, 1}, {demon, 1}})); env(signers(alice, 1, {{bogie, 1}, {demon, 1}}));
env.close(); env.close();
env.require(owners(alice, features[featureMultiSignReserve] ? 1 : 4)); env.require(owners(alice, 1));
msig phantoms{bogie, demon}; msig phantoms{bogie, demon};
std::reverse(phantoms.signers.begin(), phantoms.signers.end()); std::reverse(phantoms.signers.begin(), phantoms.signers.end());
@@ -382,7 +371,7 @@ public:
// Attach signers to alice // Attach signers to alice
env(signers(alice, 4, {{becky, 3}, {cheri, 4}}), sig(alice)); env(signers(alice, 4, {{becky, 3}, {cheri, 4}}), sig(alice));
env.close(); env.close();
env.require(owners(alice, features[featureMultiSignReserve] ? 1 : 4)); env.require(owners(alice, 1));
// Attempt a multisigned transaction that meets the quorum. // Attempt a multisigned transaction that meets the quorum.
auto const baseFee = env.current()->fees().base; auto const baseFee = env.current()->fees().base;
@@ -751,7 +740,7 @@ public:
env(signers(alice, 1, {{becky, 1}, {cheri, 1}, {daria, 1}, {jinni, 1}}), env(signers(alice, 1, {{becky, 1}, {cheri, 1}, {daria, 1}, {jinni, 1}}),
sig(alie)); sig(alie));
env.close(); env.close();
env.require(owners(alice, features[featureMultiSignReserve] ? 1 : 6)); env.require(owners(alice, 1));
// Each type of signer should succeed individually. // Each type of signer should succeed individually.
auto const baseFee = env.current()->fees().base; auto const baseFee = env.current()->fees().base;
@@ -798,7 +787,7 @@ public:
{jinni, 0xFFFF}}), {jinni, 0xFFFF}}),
sig(alie)); sig(alie));
env.close(); env.close();
env.require(owners(alice, features[featureMultiSignReserve] ? 1 : 6)); env.require(owners(alice, 1));
aliceSeq = env.seq(alice); aliceSeq = env.seq(alice);
env(noop(alice), env(noop(alice),
@@ -829,7 +818,7 @@ public:
{spook, 0xFFFF}}), {spook, 0xFFFF}}),
sig(alie)); sig(alie));
env.close(); env.close();
env.require(owners(alice, features[featureMultiSignReserve] ? 1 : 10)); env.require(owners(alice, 1));
aliceSeq = env.seq(alice); aliceSeq = env.seq(alice);
env(noop(alice), env(noop(alice),
@@ -1005,8 +994,7 @@ public:
// Attach signers to alice. // Attach signers to alice.
env(signers(alice, 2, {{becky, 1}, {bogie, 1}}), sig(alie)); env(signers(alice, 2, {{becky, 1}, {bogie, 1}}), sig(alie));
env.close(); env.close();
int const signerListOwners{features[featureMultiSignReserve] ? 1 : 4}; env.require(owners(alice, 1));
env.require(owners(alice, signerListOwners + 0));
// Multisign a ttPAYMENT. // Multisign a ttPAYMENT.
auto const baseFee = env.current()->fees().base; auto const baseFee = env.current()->fees().base;
@@ -1036,7 +1024,7 @@ public:
fee(3 * baseFee), fee(3 * baseFee),
require(lines("alice", 1))); require(lines("alice", 1)));
env.close(); env.close();
env.require(owners(alice, signerListOwners + 1)); env.require(owners(alice, 2));
// Multisign a ttOFFER_CREATE transaction. // Multisign a ttOFFER_CREATE transaction.
env(pay(gw, alice, USD(50))); env(pay(gw, alice, USD(50)));
@@ -1049,7 +1037,7 @@ public:
msig(becky, bogie), msig(becky, bogie),
fee(3 * baseFee)); fee(3 * baseFee));
env.close(); env.close();
env.require(owners(alice, signerListOwners + 2)); env.require(owners(alice, 3));
// Now multisign a ttOFFER_CANCEL canceling the offer we just created. // Now multisign a ttOFFER_CANCEL canceling the offer we just created.
{ {
@@ -1060,7 +1048,7 @@ public:
fee(3 * baseFee)); fee(3 * baseFee));
env.close(); env.close();
BEAST_EXPECT(env.seq(alice) == aliceSeq + 1); BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
env.require(owners(alice, signerListOwners + 1)); env.require(owners(alice, 2));
} }
// Multisign a ttSIGNER_LIST_SET. // Multisign a ttSIGNER_LIST_SET.
@@ -1068,7 +1056,7 @@ public:
msig(becky, bogie), msig(becky, bogie),
fee(3 * baseFee)); fee(3 * baseFee));
env.close(); env.close();
env.require(owners(alice, features[featureMultiSignReserve] ? 2 : 6)); env.require(owners(alice, 2));
} }
void void
@@ -1178,56 +1166,44 @@ public:
"fails local checks: Invalid Signers array size."); "fails local checks: Invalid Signers array size.");
} }
{ {
// Multisign 9 (!ExpandedSignerList) | 33 (ExpandedSignerList) times
// should fail.
JTx tx = env.jt( JTx tx = env.jt(
noop(alice), noop(alice),
fee(2 * baseFee), fee(2 * baseFee),
features[featureExpandedSignerList] ? msig( 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, 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));
STTx local = *(tx.stx); STTx local = *(tx.stx);
auto const info = submitSTTx(local); auto const info = submitSTTx(local);
BEAST_EXPECT( BEAST_EXPECT(
@@ -1446,101 +1422,6 @@ public:
.asString() == "tesSUCCESS"); .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 void
testSignersWithTickets(FeatureBitset features) testSignersWithTickets(FeatureBitset features)
{ {
@@ -1585,9 +1466,6 @@ public:
void void
testSignersWithTags(FeatureBitset features) testSignersWithTags(FeatureBitset features)
{ {
if (!features[featureExpandedSignerList])
return;
testcase("Signers With Tags"); testcase("Signers With Tags");
using namespace jtx; using namespace jtx;
@@ -1609,7 +1487,7 @@ public:
// Attach phantom signers to alice and use them for a transaction. // Attach phantom signers to alice and use them for a transaction.
env(signers(alice, 1, {{bogie, 1, bogie_tag}, {demon, 1, demon_tag}})); env(signers(alice, 1, {{bogie, 1, bogie_tag}, {demon, 1, demon_tag}}));
env.close(); env.close();
env.require(owners(alice, features[featureMultiSignReserve] ? 1 : 4)); env.require(owners(alice, 1));
// This should work. // This should work.
auto const baseFee = env.current()->fees().base; auto const baseFee = env.current()->fees().base;
@@ -1744,12 +1622,6 @@ public:
using namespace jtx; using namespace jtx;
auto const all = testable_amendments(); 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); testAll(all);
testSignerListSetFlags(all - fixInvalidTxFlags); testSignerListSetFlags(all - fixInvalidTxFlags);
@@ -1757,8 +1629,6 @@ public:
testSignerListObject(all - fixIncludeKeyletFields); testSignerListObject(all - fixIncludeKeyletFields);
testSignerListObject(all); testSignerListObject(all);
testAmendmentTransition();
} }
}; };

15
src/test/app/Oracle_test.cpp
View File

@@ -727,13 +727,13 @@ private:
} }
void void
testMultisig(FeatureBitset features) testMultisig()
{ {
testcase("Multisig"); testcase("Multisig");
using namespace jtx; using namespace jtx;
Oracle::setFee(100'000); Oracle::setFee(100'000);
Env env(*this, features); Env env(*this);
auto const baseFee = auto const baseFee =
static_cast<int>(env.current()->fees().base.drops()); static_cast<int>(env.current()->fees().base.drops());
@@ -753,9 +753,8 @@ private:
// Attach signers to alice. // Attach signers to alice.
env(signers(alice, 2, {{becky, 1}, {bogie, 1}, {ed, 2}}), sig(alie)); env(signers(alice, 2, {{becky, 1}, {bogie, 1}, {ed, 2}}), sig(alie));
env.close(); env.close();
// if multiSignReserve disabled then its 2 + 1 per signer
int const signerListOwners{features[featureMultiSignReserve] ? 1 : 5}; env.require(owners(alice, 1));
env.require(owners(alice, signerListOwners));
// Create // Create
// Force close (true) and time advancement because the close time // Force close (true) and time advancement because the close time
@@ -860,11 +859,7 @@ public:
testDelete(); testDelete();
testUpdate(); testUpdate();
testAmendment(); testAmendment();
for (auto const& features : testMultisig();
{all,
all - featureMultiSignReserve - featureExpandedSignerList,
all - featureExpandedSignerList})
testMultisig(features);
} }
}; };

19
src/test/jtx/Env_test.cpp
View File

@@ -798,17 +798,16 @@ public:
{ {
// a Env FeatureBitset has *only* those features // 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); BEAST_EXPECT(env.app().config().features.size() == 2);
foreachFeature(supported, [&](uint256 const& f) { foreachFeature(supported, [&](uint256 const& f) {
bool const has = bool const has = (f == featureDynamicMPT || f == featureFlow);
(f == featureMultiSignReserve || f == featureFlow);
this->BEAST_EXPECT(has == hasFeature(env, f)); this->BEAST_EXPECT(has == hasFeature(env, f));
}); });
} }
auto const missingSomeFeatures = auto const missingSomeFeatures =
testable_amendments() - featureMultiSignReserve - featureFlow; testable_amendments() - featureDynamicMPT - featureFlow;
BEAST_EXPECT(missingSomeFeatures.count() == (supported.count() - 2)); BEAST_EXPECT(missingSomeFeatures.count() == (supported.count() - 2));
{ {
// a Env supported_features_except is missing *only* those features // a Env supported_features_except is missing *only* those features
@@ -816,8 +815,7 @@ public:
BEAST_EXPECT( BEAST_EXPECT(
env.app().config().features.size() == (supported.count() - 2)); env.app().config().features.size() == (supported.count() - 2));
foreachFeature(supported, [&](uint256 const& f) { foreachFeature(supported, [&](uint256 const& f) {
bool hasnot = bool hasnot = (f == featureDynamicMPT || f == featureFlow);
(f == featureMultiSignReserve || f == featureFlow);
this->BEAST_EXPECT(hasnot != hasFeature(env, f)); this->BEAST_EXPECT(hasnot != hasFeature(env, f));
}); });
} }
@@ -829,8 +827,8 @@ public:
// the two supported ones // the two supported ones
Env env{ Env env{
*this, *this,
FeatureBitset( FeatureBitset{
featureMultiSignReserve, featureFlow, *neverSupportedFeat)}; featureDynamicMPT, featureFlow, *neverSupportedFeat}};
// this app will have just 2 supported amendments and // this app will have just 2 supported amendments and
// one additional never supported feature flag // one additional never supported feature flag
@@ -838,7 +836,7 @@ public:
BEAST_EXPECT(hasFeature(env, *neverSupportedFeat)); BEAST_EXPECT(hasFeature(env, *neverSupportedFeat));
foreachFeature(supported, [&](uint256 const& f) { foreachFeature(supported, [&](uint256 const& f) {
bool has = (f == featureMultiSignReserve || f == featureFlow); bool has = (f == featureDynamicMPT || f == featureFlow);
this->BEAST_EXPECT(has == hasFeature(env, f)); this->BEAST_EXPECT(has == hasFeature(env, f));
}); });
} }
@@ -858,8 +856,7 @@ public:
(supported.count() - 2 + 1)); (supported.count() - 2 + 1));
BEAST_EXPECT(hasFeature(env, *neverSupportedFeat)); BEAST_EXPECT(hasFeature(env, *neverSupportedFeat));
foreachFeature(supported, [&](uint256 const& f) { foreachFeature(supported, [&](uint256 const& f) {
bool hasnot = bool hasnot = (f == featureDynamicMPT || f == featureFlow);
(f == featureMultiSignReserve || f == featureFlow);
this->BEAST_EXPECT(hasnot != hasFeature(env, f)); this->BEAST_EXPECT(hasnot != hasFeature(env, f));
}); });
} }

2
src/test/protocol/STTx_test.cpp
View File

@@ -1612,7 +1612,7 @@ public:
// proper lifetime. // proper lifetime.
std::unordered_set<uint256, beast::uhash<>> const presets; std::unordered_set<uint256, beast::uhash<>> const presets;
Rules const defaultRules{presets}; Rules const defaultRules{presets};
BEAST_EXPECT(!defaultRules.enabled(featureExpandedSignerList)); BEAST_EXPECT(!defaultRules.enabled(featureAMM));
unexpected( unexpected(
!j.checkSign(STTx::RequireFullyCanonicalSig::yes, defaultRules), !j.checkSign(STTx::RequireFullyCanonicalSig::yes, defaultRules),

14
src/test/rpc/Feature_test.cpp
View File

@@ -183,16 +183,16 @@ class Feature_test : public beast::unit_test::suite
using namespace test::jtx; using namespace test::jtx;
Env env{*this}; 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"); BEAST_EXPECTS(jrr[jss::status] == jss::success, "status");
jrr.removeMember(jss::status); jrr.removeMember(jss::status);
BEAST_EXPECT(jrr.size() == 1); BEAST_EXPECT(jrr.size() == 1);
BEAST_EXPECT( BEAST_EXPECT(
jrr.isMember("586480873651E106F1D6339B0C4A8945BA705A777F3F4524626FF" jrr.isMember("00C1FC4A53E60AB02C864641002B3172F38677E29C26C54066851"
"1FC07EFE41D")); "79B37E1EDAC"));
auto feature = *(jrr.begin()); 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::enabled].asBool(), "enabled");
BEAST_EXPECTS( BEAST_EXPECTS(
feature[jss::vetoed].isBool() && !feature[jss::vetoed].asBool(), 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"); BEAST_EXPECTS(feature[jss::supported].asBool(), "supported");
// feature names are case-sensitive - expect error here // 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] == "badFeature");
BEAST_EXPECT(jrr[jss::error_message] == "Feature unknown or invalid."); BEAST_EXPECT(jrr[jss::error_message] == "Feature unknown or invalid.");
} }
@@ -476,8 +476,8 @@ class Feature_test : public beast::unit_test::suite
testcase("Veto"); testcase("Veto");
using namespace test::jtx; using namespace test::jtx;
Env env{*this, FeatureBitset(featureMultiSignReserve)}; Env env{*this, FeatureBitset{featureRequireFullyCanonicalSig}};
constexpr char const* featureName = "MultiSignReserve"; constexpr char const* featureName = "RequireFullyCanonicalSig";
auto jrr = env.rpc("feature", featureName)[jss::result]; auto jrr = env.rpc("feature", featureName)[jss::result];
if (!BEAST_EXPECTS(jrr[jss::status] == jss::success, "status")) if (!BEAST_EXPECTS(jrr[jss::status] == jss::success, "status"))

40
src/xrpld/app/tx/detail/SetSignerList.cpp
View File

@@ -142,10 +142,6 @@ SetSignerList::preCompute()
// The return type is signed so it is compatible with the 3rd argument // The return type is signed so it is compatible with the 3rd argument
// of adjustOwnerCount() (which must be signed). // 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 static int
signerCountBasedOwnerCountDelta(std::size_t entryCount, Rules const& rules) 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. // units. A SignerList with 8 entries would cost 10 OwnerCount units.
// //
// The static_cast should always be safe since entryCount should always // 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. // We've got a lot of room to grow.
XRPL_ASSERT( XRPL_ASSERT(
entryCount >= STTx::minMultiSigners, entryCount >= STTx::minMultiSigners,
"ripple::signerCountBasedOwnerCountDelta : minimum signers"); "ripple::signerCountBasedOwnerCountDelta : minimum signers");
XRPL_ASSERT( XRPL_ASSERT(
entryCount <= STTx::maxMultiSigners(&rules), entryCount <= STTx::maxMultiSigners,
"ripple::signerCountBasedOwnerCountDelta : maximum signers"); "ripple::signerCountBasedOwnerCountDelta : maximum signers");
return 2 + static_cast<int>(entryCount); return 2 + static_cast<int>(entryCount);
} }
@@ -250,8 +246,8 @@ SetSignerList::validateQuorumAndSignerEntries(
// Reject if there are too many or too few entries in the list. // Reject if there are too many or too few entries in the list.
{ {
std::size_t const signerCount = signers.size(); std::size_t const signerCount = signers.size();
if ((signerCount < STTx::minMultiSigners) || if (signerCount < STTx::minMultiSigners ||
(signerCount > STTx::maxMultiSigners(&rules))) signerCount > STTx::maxMultiSigners)
{ {
JLOG(j.trace()) << "Too many or too few signers in signer list."; JLOG(j.trace()) << "Too many or too few signers in signer list.";
return temMALFORMED; return temMALFORMED;
@@ -269,9 +265,6 @@ SetSignerList::validateQuorumAndSignerEntries(
return temBAD_SIGNER; 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 // Make sure no signers reference this account. Also make sure the
// quorum can be reached. // quorum can be reached.
std::uint64_t allSignersWeight(0); std::uint64_t allSignersWeight(0);
@@ -291,15 +284,6 @@ SetSignerList::validateQuorumAndSignerEntries(
JLOG(j.trace()) << "A signer may not self reference account."; JLOG(j.trace()) << "A signer may not self reference account.";
return temBAD_SIGNER; 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 // Don't verify that the signer accounts exist. Non-existent accounts
// may be phantom accounts (which are permitted). // 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. // Compute new reserve. Verify the account has funds to meet the reserve.
std::uint32_t const oldOwnerCount{(*sle)[sfOwnerCount]}; std::uint32_t const oldOwnerCount{(*sle)[sfOwnerCount]};
// The required reserve changes based on featureMultiSignReserve... constexpr int addedOwnerCount = 1;
int addedOwnerCount{1};
std::uint32_t flags{lsfOneOwnerCount}; std::uint32_t flags{lsfOneOwnerCount};
if (!ctx_.view().rules().enabled(featureMultiSignReserve))
{
addedOwnerCount = signerCountBasedOwnerCountDelta(
signers_.size(), ctx_.view().rules());
flags = 0;
}
XRPAmount const newReserve{ XRPAmount const newReserve{
view().fees().accountReserve(oldOwnerCount + addedOwnerCount)}; view().fees().accountReserve(oldOwnerCount + addedOwnerCount)};
@@ -415,9 +392,6 @@ SetSignerList::writeSignersToSLE(
if (flags) // Only set flags if they are non-default (default is zero). if (flags) // Only set flags if they are non-default (default is zero).
ledgerEntry->setFieldU32(sfFlags, flags); ledgerEntry->setFieldU32(sfFlags, flags);
bool const expandedSignerList =
ctx_.view().rules().enabled(featureExpandedSignerList);
// Create the SignerListArray one SignerEntry at a time. // Create the SignerListArray one SignerEntry at a time.
STArray toLedger(signers_.size()); STArray toLedger(signers_.size());
for (auto const& entry : signers_) for (auto const& entry : signers_)
@@ -429,8 +403,8 @@ SetSignerList::writeSignersToSLE(
obj[sfSignerWeight] = entry.weight; obj[sfSignerWeight] = entry.weight;
// This is a defensive check to make absolutely sure we will never write // This is a defensive check to make absolutely sure we will never write
// a tag into the ledger while featureExpandedSignerList is not enabled // a tag into the ledger.
if (expandedSignerList && entry.tag) if (entry.tag)
obj.setFieldH256(sfWalletLocator, *(entry.tag)); obj.setFieldH256(sfWalletLocator, *(entry.tag));
} }

2
src/xrpld/app/tx/detail/SignerEntries.cpp
View File

@@ -25,7 +25,7 @@ SignerEntries::deserialize(
} }
std::vector<SignerEntry> accountVec; std::vector<SignerEntry> accountVec;
accountVec.reserve(STTx::maxMultiSigners()); accountVec.reserve(STTx::maxMultiSigners);
STArray const& sEntries(obj.getFieldArray(sfSignerEntries)); STArray const& sEntries(obj.getFieldArray(sfSignerEntries));
for (STObject const& sEntry : sEntries) for (STObject const& sEntry : sEntries)

2
src/xrpld/rpc/detail/TransactionSign.cpp
View File

@@ -822,7 +822,7 @@ getTxFee(Application const& app, Config const& config, Json::Value tx)
if (!tx[jss::Signers].isArray()) if (!tx[jss::Signers].isArray())
return config.FEES.reference_fee; 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; return config.FEES.reference_fee;
// check multi-signed signers // check multi-signed signers