Skip to content

Commit

Permalink
Introduce the ExpandedSignerList amendment:
Browse files Browse the repository at this point in the history
The amendment increases the maximum sign of an account's signer
list from 8 to 32.

Like all new features, the associated amendment is configured with
a default vote of "no" and server operators will have to vote for
it explicitly if they believe it is useful.
  • Loading branch information
RichardAH authored and manojsdoshi committed May 10, 2022
1 parent 04bd587 commit 01c37fe
Show file tree
Hide file tree
Showing 24 changed files with 506 additions and 215 deletions.
2 changes: 2 additions & 0 deletions Builds/CMake/RippledCore.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ target_sources (xrpl_core PRIVATE
src/ripple/protocol/impl/PublicKey.cpp
src/ripple/protocol/impl/Quality.cpp
src/ripple/protocol/impl/Rate2.cpp
src/ripple/protocol/impl/Rules.cpp
src/ripple/protocol/impl/SField.cpp
src/ripple/protocol/impl/SOTemplate.cpp
src/ripple/protocol/impl/STAccount.cpp
Expand Down Expand Up @@ -209,6 +210,7 @@ install (
src/ripple/protocol/PublicKey.h
src/ripple/protocol/Quality.h
src/ripple/protocol/Rate.h
src/ripple/protocol/Rules.h
src/ripple/protocol/SField.h
src/ripple/protocol/SOTemplate.h
src/ripple/protocol/STAccount.h
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/app/consensus/RCLConsensus.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,7 @@ RCLConsensus::Adaptor::doAccept(
auto const lastVal = ledgerMaster_.getValidatedLedger();
std::optional<Rules> rules;
if (lastVal)
rules.emplace(*lastVal, app_.config().features);
rules = makeRulesGivenLedger(*lastVal, app_.config().features);
else
rules.emplace(app_.config().features);
app_.openLedger().accept(
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/app/ledger/Ledger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -626,7 +626,7 @@ Ledger::setup(Config const& config)

try
{
rules_ = Rules(*this, config.features);
rules_ = makeRulesGivenLedger(*this, config.features);
}
catch (SHAMapMissingNode const&)
{
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/app/misc/NetworkOPs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1749,7 +1749,7 @@ NetworkOPsImp::switchLastClosedLedger(
auto const lastVal = app_.getLedgerMaster().getValidatedLedger();
std::optional<Rules> rules;
if (lastVal)
rules.emplace(*lastVal, app_.config().features);
rules = makeRulesGivenLedger(*lastVal, app_.config().features);
else
rules.emplace(app_.config().features);
app_.openLedger().accept(
Expand Down
44 changes: 36 additions & 8 deletions src/ripple/app/tx/impl/SetSignerList.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ SetSignerList::preflight(PreflightContext const& ctx)
return ret;

auto const result = determineOperation(ctx.tx, ctx.flags, ctx.j);

if (std::get<0>(result) != tesSUCCESS)
return std::get<0>(result);

Expand All @@ -98,7 +99,11 @@ SetSignerList::preflight(PreflightContext const& ctx)
// Validate our settings.
auto const account = ctx.tx.getAccountID(sfAccount);
NotTEC const ter = validateQuorumAndSignerEntries(
std::get<1>(result), std::get<2>(result), account, ctx.j);
std::get<1>(result),
std::get<2>(result),
account,
ctx.j,
ctx.rules);
if (ter != tesSUCCESS)
{
return ter;
Expand Down Expand Up @@ -149,7 +154,7 @@ SetSignerList::preCompute()
// 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)
signerCountBasedOwnerCountDelta(std::size_t entryCount, Rules const& rules)
{
// We always compute the full change in OwnerCount, taking into account:
// o The fact that we're adding/removing a SignerList and
Expand All @@ -164,9 +169,10 @@ signerCountBasedOwnerCountDelta(std::size_t entryCount)
// 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. We've got a lot of room to grow.
// be in the range from 1 to 8 (or 32 if ExpandedSignerList is enabled).
// We've got a lot of room to grow.
assert(entryCount >= STTx::minMultiSigners);
assert(entryCount <= STTx::maxMultiSigners);
assert(entryCount <= STTx::maxMultiSigners(&rules));
return 2 + static_cast<int>(entryCount);
}

Expand Down Expand Up @@ -195,7 +201,8 @@ removeSignersFromLedger(
{
STArray const& actualList = signers->getFieldArray(sfSignerEntries);
removeFromOwnerCount =
signerCountBasedOwnerCountDelta(actualList.size()) * -1;
signerCountBasedOwnerCountDelta(actualList.size(), view.rules()) *
-1;
}

// Remove the node from the account directory.
Expand Down Expand Up @@ -238,13 +245,14 @@ SetSignerList::validateQuorumAndSignerEntries(
std::uint32_t quorum,
std::vector<SignerEntries::SignerEntry> const& signers,
AccountID const& account,
beast::Journal j)
beast::Journal j,
Rules const& rules)
{
// 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))
(signerCount > STTx::maxMultiSigners(&rules)))
{
JLOG(j.trace()) << "Too many or too few signers in signer list.";
return temMALFORMED;
Expand All @@ -259,6 +267,9 @@ 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);
Expand All @@ -279,6 +290,14 @@ SetSignerList::validateQuorumAndSignerEntries(
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).
}
Expand Down Expand Up @@ -321,7 +340,8 @@ SetSignerList::replaceSignerList()
std::uint32_t flags{lsfOneOwnerCount};
if (!ctx_.view().rules().enabled(featureMultiSignReserve))
{
addedOwnerCount = signerCountBasedOwnerCountDelta(signers_.size());
addedOwnerCount = signerCountBasedOwnerCountDelta(
signers_.size(), ctx_.view().rules());
flags = 0;
}

Expand Down Expand Up @@ -389,6 +409,9 @@ 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_)
Expand All @@ -398,6 +421,11 @@ SetSignerList::writeSignersToSLE(
obj.reserve(2);
obj.setAccountID(sfAccount, entry.account);
obj.setFieldU16(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)
obj.setFieldH256(sfWalletLocator, *(entry.tag));
}

// Assign the SignerEntries.
Expand Down
4 changes: 3 additions & 1 deletion src/ripple/app/tx/impl/SetSignerList.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include <ripple/app/tx/impl/Transactor.h>
#include <ripple/basics/Log.h>
#include <ripple/protocol/Indexes.h>
#include <ripple/protocol/Rules.h>
#include <ripple/protocol/STArray.h>
#include <ripple/protocol/STObject.h>
#include <ripple/protocol/STTx.h>
Expand Down Expand Up @@ -83,7 +84,8 @@ class SetSignerList : public Transactor
std::uint32_t quorum,
std::vector<SignerEntries::SignerEntry> const& signers,
AccountID const& account,
beast::Journal j);
beast::Journal j,
Rules const&);

TER
replaceSignerList();
Expand Down
7 changes: 5 additions & 2 deletions src/ripple/app/tx/impl/SignerEntries.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <ripple/protocol/STArray.h>
#include <ripple/protocol/STObject.h>
#include <cstdint>
#include <optional>

namespace ripple {

Expand All @@ -41,7 +42,7 @@ SignerEntries::deserialize(
}

std::vector<SignerEntry> accountVec;
accountVec.reserve(STTx::maxMultiSigners);
accountVec.reserve(STTx::maxMultiSigners());

STArray const& sEntries(obj.getFieldArray(sfSignerEntries));
for (STObject const& sEntry : sEntries)
Expand All @@ -57,7 +58,9 @@ SignerEntries::deserialize(
// Extract SignerEntry fields.
AccountID const account = sEntry.getAccountID(sfAccount);
std::uint16_t const weight = sEntry.getFieldU16(sfSignerWeight);
accountVec.emplace_back(account, weight);
std::optional<uint256> const tag = sEntry.at(~sfWalletLocator);

accountVec.emplace_back(account, weight, tag);
}
return accountVec;
}
Expand Down
10 changes: 8 additions & 2 deletions src/ripple/app/tx/impl/SignerEntries.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,11 @@
#include <ripple/app/tx/impl/Transactor.h> // NotTEC
#include <ripple/basics/Expected.h> //
#include <ripple/beast/utility/Journal.h> // beast::Journal
#include <ripple/protocol/Rules.h> // Rules
#include <ripple/protocol/STTx.h> // STTx::maxMultiSigners
#include <ripple/protocol/TER.h> // temMALFORMED
#include <ripple/protocol/UintTypes.h> // AccountID
#include <optional>

namespace ripple {

Expand All @@ -42,9 +44,13 @@ class SignerEntries
{
AccountID account;
std::uint16_t weight;
std::optional<uint256> tag;

SignerEntry(AccountID const& inAccount, std::uint16_t inWeight)
: account(inAccount), weight(inWeight)
SignerEntry(
AccountID const& inAccount,
std::uint16_t inWeight,
std::optional<uint256> inTag)
: account(inAccount), weight(inWeight), tag(inTag)
{
}

Expand Down
2 changes: 1 addition & 1 deletion src/ripple/app/tx/impl/apply.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ checkValidity(
? STTx::RequireFullyCanonicalSig::yes
: STTx::RequireFullyCanonicalSig::no;

auto const sigVerify = tx.checkSign(requireCanonicalSig);
auto const sigVerify = tx.checkSign(requireCanonicalSig, rules);
if (!sigVerify)
{
router.setFlags(id, SF_SIGBAD);
Expand Down
64 changes: 6 additions & 58 deletions src/ripple/ledger/ReadView.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include <ripple/ledger/detail/ReadViewFwdRange.h>
#include <ripple/protocol/Indexes.h>
#include <ripple/protocol/Protocol.h>
#include <ripple/protocol/Rules.h>
#include <ripple/protocol/STAmount.h>
#include <ripple/protocol/STLedgerEntry.h>
#include <ripple/protocol/STTx.h>
Expand Down Expand Up @@ -125,64 +126,6 @@ struct LedgerInfo

//------------------------------------------------------------------------------

class DigestAwareReadView;

/** Rules controlling protocol behavior. */
class Rules
{
private:
class Impl;

std::shared_ptr<Impl const> impl_;

public:
Rules(Rules const&) = default;
Rules&
operator=(Rules const&) = default;

Rules() = delete;

/** Construct an empty rule set.
These are the rules reflected by
the genesis ledger.
*/
explicit Rules(std::unordered_set<uint256, beast::uhash<>> const& presets);

/** Construct rules from a ledger.
The ledger contents are analyzed for rules
and amendments and extracted to the object.
*/
explicit Rules(
DigestAwareReadView const& ledger,
std::unordered_set<uint256, beast::uhash<>> const& presets);

/** Returns `true` if a feature is enabled. */
bool
enabled(uint256 const& id) const;

/** Returns `true` if these rules don't match the ledger. */
bool
changed(DigestAwareReadView const& ledger) const;

/** Returns `true` if two rule sets are identical.
@note This is for diagnostics. To determine if new
rules should be constructed, call changed() first instead.
*/
bool
operator==(Rules const&) const;

bool
operator!=(Rules const& other) const
{
return !(*this == other);
}
};

//------------------------------------------------------------------------------

/** A view into a ledger.
This interface provides read access to state
Expand Down Expand Up @@ -423,6 +366,11 @@ getCloseAgree(LedgerInfo const& info)
void
addRaw(LedgerInfo const&, Serializer&, bool includeHash = false);

Rules
makeRulesGivenLedger(
DigestAwareReadView const& ledger,
std::unordered_set<uint256, beast::uhash<>> const& presets);

} // namespace ripple

#include <ripple/ledger/detail/ReadViewFwdRange.ipp>
Expand Down
Loading

1 comment on commit 01c37fe

@intelliot
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.