Skip to content

Commit

Permalink
add featureExpandedSignerList
Browse files Browse the repository at this point in the history
  • Loading branch information
RichardAH committed Mar 4, 2022
1 parent 72377e7 commit 33c236b
Show file tree
Hide file tree
Showing 21 changed files with 483 additions and 210 deletions.
1 change: 1 addition & 0 deletions Builds/CMake/RippledCore.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,7 @@ target_sources (rippled PRIVATE
src/ripple/ledger/impl/RawStateTable.cpp
src/ripple/ledger/impl/ReadView.cpp
src/ripple/ledger/impl/View.cpp
src/ripple/ledger/impl/Rules.cpp
#[===============================[
main sources:
subdir: net
Expand Down
3 changes: 3 additions & 0 deletions Builds/levelization/results/loops.txt
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ Loop: ripple.basics ripple.rpc
Loop: ripple.core ripple.net
ripple.net > ripple.core

Loop: ripple.ledger ripple.protocol
ripple.ledger > ripple.protocol

Loop: ripple.net ripple.rpc
ripple.rpc > ripple.net

Expand Down
2 changes: 1 addition & 1 deletion Builds/levelization/results/ordering.txt
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ ripple.ledger > ripple.basics
ripple.ledger > ripple.beast
ripple.ledger > ripple.core
ripple.ledger > ripple.json
ripple.ledger > ripple.protocol
ripple.net > ripple.basics
ripple.net > ripple.beast
ripple.net > ripple.json
Expand Down Expand Up @@ -178,6 +177,7 @@ test.protocol > ripple.basics
test.protocol > ripple.beast
test.protocol > ripple.crypto
test.protocol > ripple.json
test.protocol > ripple.ledger
test.protocol > ripple.protocol
test.protocol > test.toplevel
test.resource > ripple.basics
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 @@ -83,6 +83,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 @@ -99,7 +100,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 @@ -150,7 +155,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 @@ -165,9 +170,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 @@ -196,7 +202,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 @@ -239,13 +246,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 @@ -260,6 +268,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 @@ -280,6 +291,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 @@ -322,7 +341,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 @@ -390,6 +410,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 @@ -399,6 +422,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 @@ -24,6 +24,7 @@
#include <ripple/app/tx/impl/SignerEntries.h>
#include <ripple/app/tx/impl/Transactor.h>
#include <ripple/basics/Log.h>
#include <ripple/ledger/Rules.h>
#include <ripple/protocol/Indexes.h>
#include <ripple/protocol/STArray.h>
#include <ripple/protocol/STObject.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
8 changes: 6 additions & 2 deletions src/ripple/app/tx/impl/SignerEntries.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@

#include <ripple/app/tx/impl/SignerEntries.h>
#include <ripple/basics/Log.h>
#include <ripple/ledger/Rules.h>
#include <ripple/protocol/STArray.h>
#include <ripple/protocol/STObject.h>
#include <cstdint>
#include <optional>

namespace ripple {

Expand All @@ -41,7 +43,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 +59,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/ledger/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
59 changes: 1 addition & 58 deletions src/ripple/ledger/ReadView.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include <ripple/basics/chrono.h>
#include <ripple/beast/hash/uhash.h>
#include <ripple/beast/utility/Journal.h>
#include <ripple/ledger/Rules.h>
#include <ripple/ledger/detail/ReadViewFwdRange.h>
#include <ripple/protocol/Indexes.h>
#include <ripple/protocol/Protocol.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
Loading

0 comments on commit 33c236b

Please sign in to comment.