From a2f9d78c57cdca3afd3c2b3b395cc978dc46628a Mon Sep 17 00:00:00 2001 From: Richard Holland Date: Mon, 11 Apr 2022 10:21:56 +0000 Subject: [PATCH] Amendment: ExpandedSignerList --- Builds/CMake/RippledCore.cmake | 2 + src/ripple/app/consensus/RCLConsensus.cpp | 2 +- src/ripple/app/ledger/Ledger.cpp | 2 +- src/ripple/app/misc/NetworkOPs.cpp | 2 +- src/ripple/app/tx/impl/SetSignerList.cpp | 44 +++- src/ripple/app/tx/impl/SetSignerList.h | 4 +- src/ripple/app/tx/impl/SignerEntries.cpp | 7 +- src/ripple/app/tx/impl/SignerEntries.h | 10 +- src/ripple/app/tx/impl/apply.cpp | 2 +- src/ripple/ledger/ReadView.h | 64 +----- src/ripple/ledger/impl/ReadView.cpp | 118 ++--------- src/ripple/proto/org/xrpl/rpc/v1/common.proto | 8 + src/ripple/protocol/Feature.h | 1 + src/ripple/protocol/Rules.h | 86 ++++++++ src/ripple/protocol/STTx.h | 20 +- src/ripple/protocol/impl/Feature.cpp | 1 + .../protocol/impl/InnerObjectFormats.cpp | 1 + src/ripple/protocol/impl/Rules.cpp | 103 +++++++++ src/ripple/protocol/impl/STTx.cpp | 16 +- src/ripple/rpc/impl/GRPCHelpers.cpp | 9 + src/test/app/MultiSign_test.cpp | 200 +++++++++++++++--- src/test/jtx/impl/multisign.cpp | 4 + src/test/jtx/multisign.h | 9 +- src/test/protocol/STTx_test.cpp | 6 +- 24 files changed, 506 insertions(+), 215 deletions(-) create mode 100644 src/ripple/protocol/Rules.h create mode 100644 src/ripple/protocol/impl/Rules.cpp diff --git a/Builds/CMake/RippledCore.cmake b/Builds/CMake/RippledCore.cmake index 35b147ad491..fbf0608d892 100644 --- a/Builds/CMake/RippledCore.cmake +++ b/Builds/CMake/RippledCore.cmake @@ -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 @@ -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 diff --git a/src/ripple/app/consensus/RCLConsensus.cpp b/src/ripple/app/consensus/RCLConsensus.cpp index be8f2af8cae..79d41581ae3 100644 --- a/src/ripple/app/consensus/RCLConsensus.cpp +++ b/src/ripple/app/consensus/RCLConsensus.cpp @@ -632,7 +632,7 @@ RCLConsensus::Adaptor::doAccept( auto const lastVal = ledgerMaster_.getValidatedLedger(); std::optional rules; if (lastVal) - rules.emplace(*lastVal, app_.config().features); + rules = makeRulesGivenLedger(*lastVal, app_.config().features); else rules.emplace(app_.config().features); app_.openLedger().accept( diff --git a/src/ripple/app/ledger/Ledger.cpp b/src/ripple/app/ledger/Ledger.cpp index 66bea568273..71b6171beff 100644 --- a/src/ripple/app/ledger/Ledger.cpp +++ b/src/ripple/app/ledger/Ledger.cpp @@ -626,7 +626,7 @@ Ledger::setup(Config const& config) try { - rules_ = Rules(*this, config.features); + rules_ = makeRulesGivenLedger(*this, config.features); } catch (SHAMapMissingNode const&) { diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index 4b44cf431c7..3fba4c8c577 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -1749,7 +1749,7 @@ NetworkOPsImp::switchLastClosedLedger( auto const lastVal = app_.getLedgerMaster().getValidatedLedger(); std::optional rules; if (lastVal) - rules.emplace(*lastVal, app_.config().features); + rules = makeRulesGivenLedger(*lastVal, app_.config().features); else rules.emplace(app_.config().features); app_.openLedger().accept( diff --git a/src/ripple/app/tx/impl/SetSignerList.cpp b/src/ripple/app/tx/impl/SetSignerList.cpp index 78409ba7145..07cc705bad1 100644 --- a/src/ripple/app/tx/impl/SetSignerList.cpp +++ b/src/ripple/app/tx/impl/SetSignerList.cpp @@ -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); @@ -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; @@ -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 @@ -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(entryCount); } @@ -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. @@ -238,13 +245,14 @@ SetSignerList::validateQuorumAndSignerEntries( std::uint32_t quorum, std::vector 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; @@ -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); @@ -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). } @@ -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; } @@ -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_) @@ -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. diff --git a/src/ripple/app/tx/impl/SetSignerList.h b/src/ripple/app/tx/impl/SetSignerList.h index 345d5940232..f8e49e4a7b0 100644 --- a/src/ripple/app/tx/impl/SetSignerList.h +++ b/src/ripple/app/tx/impl/SetSignerList.h @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -83,7 +84,8 @@ class SetSignerList : public Transactor std::uint32_t quorum, std::vector const& signers, AccountID const& account, - beast::Journal j); + beast::Journal j, + Rules const&); TER replaceSignerList(); diff --git a/src/ripple/app/tx/impl/SignerEntries.cpp b/src/ripple/app/tx/impl/SignerEntries.cpp index 5081dc3e2f2..a948b2f902b 100644 --- a/src/ripple/app/tx/impl/SignerEntries.cpp +++ b/src/ripple/app/tx/impl/SignerEntries.cpp @@ -22,6 +22,7 @@ #include #include #include +#include namespace ripple { @@ -41,7 +42,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) @@ -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 const tag = sEntry.at(~sfWalletLocator); + + accountVec.emplace_back(account, weight, tag); } return accountVec; } diff --git a/src/ripple/app/tx/impl/SignerEntries.h b/src/ripple/app/tx/impl/SignerEntries.h index 96b5e29d9c8..cf4921ecf4b 100644 --- a/src/ripple/app/tx/impl/SignerEntries.h +++ b/src/ripple/app/tx/impl/SignerEntries.h @@ -23,9 +23,11 @@ #include // NotTEC #include // #include // beast::Journal +#include // Rules #include // STTx::maxMultiSigners #include // temMALFORMED #include // AccountID +#include namespace ripple { @@ -42,9 +44,13 @@ class SignerEntries { AccountID account; std::uint16_t weight; + std::optional tag; - SignerEntry(AccountID const& inAccount, std::uint16_t inWeight) - : account(inAccount), weight(inWeight) + SignerEntry( + AccountID const& inAccount, + std::uint16_t inWeight, + std::optional inTag) + : account(inAccount), weight(inWeight), tag(inTag) { } diff --git a/src/ripple/app/tx/impl/apply.cpp b/src/ripple/app/tx/impl/apply.cpp index 332364863c1..cc1e792c014 100644 --- a/src/ripple/app/tx/impl/apply.cpp +++ b/src/ripple/app/tx/impl/apply.cpp @@ -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); diff --git a/src/ripple/ledger/ReadView.h b/src/ripple/ledger/ReadView.h index bc9c0906a45..714f8dc945d 100644 --- a/src/ripple/ledger/ReadView.h +++ b/src/ripple/ledger/ReadView.h @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -125,64 +126,6 @@ struct LedgerInfo //------------------------------------------------------------------------------ -class DigestAwareReadView; - -/** Rules controlling protocol behavior. */ -class Rules -{ -private: - class Impl; - - std::shared_ptr 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> 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> 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 @@ -423,6 +366,11 @@ getCloseAgree(LedgerInfo const& info) void addRaw(LedgerInfo const&, Serializer&, bool includeHash = false); +Rules +makeRulesGivenLedger( + DigestAwareReadView const& ledger, + std::unordered_set> const& presets); + } // namespace ripple #include diff --git a/src/ripple/ledger/impl/ReadView.cpp b/src/ripple/ledger/impl/ReadView.cpp index 77db253aa16..57af008b47c 100644 --- a/src/ripple/ledger/impl/ReadView.cpp +++ b/src/ripple/ledger/impl/ReadView.cpp @@ -21,108 +21,6 @@ namespace ripple { -class Rules::Impl -{ -private: - std::unordered_set> set_; - std::optional digest_; - std::unordered_set> const& presets_; - -public: - explicit Impl(std::unordered_set> const& presets) - : presets_(presets) - { - } - - explicit Impl( - DigestAwareReadView const& ledger, - std::unordered_set> const& presets) - : presets_(presets) - { - auto const k = keylet::amendments(); - digest_ = ledger.digest(k.key); - if (!digest_) - return; - auto const sle = ledger.read(k); - if (!sle) - { - // LogicError() ? - return; - } - - for (auto const& item : sle->getFieldV256(sfAmendments)) - set_.insert(item); - } - - bool - enabled(uint256 const& feature) const - { - if (presets_.count(feature) > 0) - return true; - return set_.count(feature) > 0; - } - - bool - changed(DigestAwareReadView const& ledger) const - { - auto const digest = ledger.digest(keylet::amendments().key); - if (!digest && !digest_) - return false; - if (!digest || !digest_) - return true; - return *digest != *digest_; - } - - bool - operator==(Impl const& other) const - { - if (!digest_ && !other.digest_) - return true; - if (!digest_ || !other.digest_) - return false; - return *digest_ == *other.digest_; - } -}; - -//------------------------------------------------------------------------------ - -Rules::Rules( - DigestAwareReadView const& ledger, - std::unordered_set> const& presets) - : impl_(std::make_shared(ledger, presets)) -{ -} - -Rules::Rules(std::unordered_set> const& presets) - : impl_(std::make_shared(presets)) -{ -} - -bool -Rules::enabled(uint256 const& id) const -{ - assert(impl_); - return impl_->enabled(id); -} - -bool -Rules::changed(DigestAwareReadView const& ledger) const -{ - assert(impl_); - return impl_->changed(ledger); -} - -bool -Rules::operator==(Rules const& other) const -{ - assert(impl_ && other.impl_); - if (impl_.get() == other.impl_.get()) - return true; - return *impl_ == *other.impl_; -} - -//------------------------------------------------------------------------------ - ReadView::sles_type::sles_type(ReadView const& view) : ReadViewFwdRange(view) { } @@ -167,4 +65,20 @@ ReadView::txs_type::end() const -> iterator return iterator(view_, view_->txsEnd()); } +Rules +makeRulesGivenLedger( + DigestAwareReadView const& ledger, + std::unordered_set> const& presets) +{ + Keylet const k = keylet::amendments(); + std::optional digest = ledger.digest(k.key); + if (digest) + { + auto const sle = ledger.read(k); + if (sle) + return Rules(presets, digest, sle->getFieldV256(sfAmendments)); + } + return Rules(presets); +} + } // namespace ripple diff --git a/src/ripple/proto/org/xrpl/rpc/v1/common.proto b/src/ripple/proto/org/xrpl/rpc/v1/common.proto index 81718b507cf..fd514cbacee 100644 --- a/src/ripple/proto/org/xrpl/rpc/v1/common.proto +++ b/src/ripple/proto/org/xrpl/rpc/v1/common.proto @@ -373,6 +373,12 @@ message RootIndex bytes value = 1; } +message WalletLocator +{ + // 32 bytes + bytes value = 1; +} + // *** Messages wrapping variable length byte arrays *** @@ -586,6 +592,8 @@ message SignerEntry Account account = 1; SignerWeight signer_weight = 2; + + WalletLocator wallet_locator = 3; } // Next field: 3 diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index 9087bec992e..67991d4ff93 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -334,6 +334,7 @@ extern uint256 const fixSTAmountCanonicalize; extern uint256 const fixRmSmallIncreasedQOffers; extern uint256 const featureCheckCashMakesTrustLine; extern uint256 const featureNonFungibleTokensV1; +extern uint256 const featureExpandedSignerList; } // namespace ripple diff --git a/src/ripple/protocol/Rules.h b/src/ripple/protocol/Rules.h new file mode 100644 index 00000000000..d8190e86a71 --- /dev/null +++ b/src/ripple/protocol/Rules.h @@ -0,0 +1,86 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2012, 2013 Ripple Labs Inc. + + Permission to use, copy, modify, and/or distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#ifndef RIPPLE_LEDGER_RULES_H_INCLUDED +#define RIPPLE_LEDGER_RULES_H_INCLUDED + +#include +#include +#include +#include + +namespace ripple { + +class DigestAwareReadView; + +/** Rules controlling protocol behavior. */ +class Rules +{ +private: + class Impl; + + // Carrying impl by shared_ptr makes Rules comparatively cheap to pass + // by value. + std::shared_ptr 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> const& presets); + +private: + // Allow a friend function to construct Rules. + friend Rules + makeRulesGivenLedger( + DigestAwareReadView const& ledger, + std::unordered_set> const& presets); + + Rules( + std::unordered_set> const& presets, + std::optional const& digest, + STVector256 const& amendments); + +public: + /** Returns `true` if a feature is enabled. */ + bool + enabled(uint256 const& feature) const; + + /** Returns `true` if two rule sets are identical. + + @note This is for diagnostics. + */ + bool + operator==(Rules const&) const; + + bool + operator!=(Rules const& other) const; +}; + +} // namespace ripple +#endif diff --git a/src/ripple/protocol/STTx.h b/src/ripple/protocol/STTx.h index ca33abf8acd..c6a9e053c3d 100644 --- a/src/ripple/protocol/STTx.h +++ b/src/ripple/protocol/STTx.h @@ -21,7 +21,9 @@ #define RIPPLE_PROTOCOL_STTX_H_INCLUDED #include +#include #include +#include #include #include #include @@ -47,7 +49,16 @@ class STTx final : public STObject, public CountedObject public: static std::size_t const minMultiSigners = 1; - static std::size_t const maxMultiSigners = 8; + + // 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(STTx const& other) = default; @@ -108,7 +119,8 @@ class STTx final : public STObject, public CountedObject */ enum class RequireFullyCanonicalSig : bool { no, yes }; Expected - checkSign(RequireFullyCanonicalSig requireCanonicalSig) const; + checkSign(RequireFullyCanonicalSig requireCanonicalSig, Rules const& rules) + const; // SQL Functions with metadata. static std::string const& @@ -130,7 +142,9 @@ class STTx final : public STObject, public CountedObject checkSingleSign(RequireFullyCanonicalSig requireCanonicalSig) const; Expected - checkMultiSign(RequireFullyCanonicalSig requireCanonicalSig) const; + checkMultiSign( + RequireFullyCanonicalSig requireCanonicalSig, + Rules const& rules) const; STBase* copy(std::size_t n, void* buf) const override; diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index d713dc8c43b..7b9161c08cd 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -438,6 +438,7 @@ REGISTER_FIX (fixSTAmountCanonicalize, Supported::yes, DefaultVote::yes REGISTER_FIX (fixRmSmallIncreasedQOffers, Supported::yes, DefaultVote::yes); REGISTER_FEATURE(CheckCashMakesTrustLine, Supported::yes, DefaultVote::no); REGISTER_FEATURE(NonFungibleTokensV1, Supported::yes, DefaultVote::no); +REGISTER_FEATURE(ExpandedSignerList, Supported::yes, DefaultVote::no); // The following amendments have been active for at least two years. Their // pre-amendment code has been removed and the identifiers are deprecated. diff --git a/src/ripple/protocol/impl/InnerObjectFormats.cpp b/src/ripple/protocol/impl/InnerObjectFormats.cpp index c1b2acc87d2..1b4aa63c2ba 100644 --- a/src/ripple/protocol/impl/InnerObjectFormats.cpp +++ b/src/ripple/protocol/impl/InnerObjectFormats.cpp @@ -28,6 +28,7 @@ InnerObjectFormats::InnerObjectFormats() { {sfAccount, soeREQUIRED}, {sfSignerWeight, soeREQUIRED}, + {sfWalletLocator, soeOPTIONAL}, }); add(sfSigner.jsonName.c_str(), diff --git a/src/ripple/protocol/impl/Rules.cpp b/src/ripple/protocol/impl/Rules.cpp new file mode 100644 index 00000000000..3736764fcf9 --- /dev/null +++ b/src/ripple/protocol/impl/Rules.cpp @@ -0,0 +1,103 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2012, 2013 Ripple Labs Inc. + + Permission to use, copy, modify, and/or distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#include + +#include + +namespace ripple { + +class Rules::Impl +{ +private: + std::unordered_set> set_; + std::optional digest_; + std::unordered_set> const& presets_; + +public: + explicit Impl(std::unordered_set> const& presets) + : presets_(presets) + { + } + + Impl( + std::unordered_set> const& presets, + std::optional const& digest, + STVector256 const& amendments) + : presets_(presets) + { + digest_ = digest; + set_.reserve(amendments.size()); + set_.insert(amendments.begin(), amendments.end()); + } + + bool + enabled(uint256 const& feature) const + { + if (presets_.count(feature) > 0) + return true; + return set_.count(feature) > 0; + } + + bool + operator==(Impl const& other) const + { + if (!digest_ && !other.digest_) + return true; + if (!digest_ || !other.digest_) + return false; + return *digest_ == *other.digest_; + } +}; + +Rules::Rules(std::unordered_set> const& presets) + : impl_(std::make_shared(presets)) +{ +} + +Rules::Rules( + std::unordered_set> const& presets, + std::optional const& digest, + STVector256 const& amendments) + : impl_(std::make_shared(presets, digest, amendments)) +{ +} + +bool +Rules::enabled(uint256 const& feature) const +{ + assert(impl_); + return impl_->enabled(feature); +} + +bool +Rules::operator==(Rules const& other) const +{ + assert(impl_ && other.impl_); + if (impl_.get() == other.impl_.get()) + return true; + return *impl_ == *other.impl_; +} + +bool +Rules::operator!=(Rules const& other) const +{ + return !(*this == other); +} +} // namespace ripple diff --git a/src/ripple/protocol/impl/STTx.cpp b/src/ripple/protocol/impl/STTx.cpp index c8e05c74211..66d20f3167a 100644 --- a/src/ripple/protocol/impl/STTx.cpp +++ b/src/ripple/protocol/impl/STTx.cpp @@ -206,7 +206,9 @@ STTx::sign(PublicKey const& publicKey, SecretKey const& secretKey) } Expected -STTx::checkSign(RequireFullyCanonicalSig requireCanonicalSig) const +STTx::checkSign( + RequireFullyCanonicalSig requireCanonicalSig, + Rules const& rules) const { try { @@ -214,8 +216,9 @@ STTx::checkSign(RequireFullyCanonicalSig requireCanonicalSig) const // at the SigningPubKey. If it's empty we must be // multi-signing. Otherwise we're single-signing. Blob const& signingPubKey = getFieldVL(sfSigningPubKey); - return signingPubKey.empty() ? checkMultiSign(requireCanonicalSig) - : checkSingleSign(requireCanonicalSig); + return signingPubKey.empty() + ? checkMultiSign(requireCanonicalSig, rules) + : checkSingleSign(requireCanonicalSig); } catch (std::exception const&) { @@ -327,7 +330,9 @@ STTx::checkSingleSign(RequireFullyCanonicalSig requireCanonicalSig) const } Expected -STTx::checkMultiSign(RequireFullyCanonicalSig requireCanonicalSig) const +STTx::checkMultiSign( + RequireFullyCanonicalSig requireCanonicalSig, + Rules const& rules) const { // Make sure the MultiSigners are present. Otherwise they are not // attempting multi-signing and we just have a bad SigningPubKey. @@ -342,7 +347,8 @@ STTx::checkMultiSign(RequireFullyCanonicalSig requireCanonicalSig) const STArray const& signers{getFieldArray(sfSigners)}; // There are well known bounds that the number of signers must be within. - if (signers.size() < minMultiSigners || signers.size() > maxMultiSigners) + if (signers.size() < minMultiSigners || + signers.size() > maxMultiSigners(&rules)) return Unexpected("Invalid Signers array size."); // We can ease the computational load inside the loop a bit by diff --git a/src/ripple/rpc/impl/GRPCHelpers.cpp b/src/ripple/rpc/impl/GRPCHelpers.cpp index 558c9d53566..e06512ce2c8 100644 --- a/src/ripple/rpc/impl/GRPCHelpers.cpp +++ b/src/ripple/rpc/impl/GRPCHelpers.cpp @@ -746,6 +746,14 @@ populateSignerListID(T& to, STObject const& from) [&to]() { return to.mutable_signer_list_id(); }, from, sfSignerListID); } +template +void +populateWalletLocator(T& to, STObject const& from) +{ + populateProtoPrimitive( + [&to]() { return to.mutable_wallet_locator(); }, from, sfWalletLocator); +} + template void populateTicketSequence(T& to, STObject const& from) @@ -1012,6 +1020,7 @@ populateSignerEntries(T& to, STObject const& from) [](auto& innerObj, auto& innerProto) { populateAccount(innerProto, innerObj); populateSignerWeight(innerProto, innerObj); + populateWalletLocator(innerProto, innerObj); }, from, sfSignerEntries, diff --git a/src/test/app/MultiSign_test.cpp b/src/test/app/MultiSign_test.cpp index 6e9278f51da..433cf2f7cd8 100644 --- a/src/test/app/MultiSign_test.cpp +++ b/src/test/app/MultiSign_test.cpp @@ -34,6 +34,30 @@ class MultiSign_test : public beast::unit_test::suite jtx::Account const phase{"phase", KeyType::ed25519}; jtx::Account const shade{"shade", KeyType::secp256k1}; jtx::Account const spook{"spook", KeyType::ed25519}; + jtx::Account const acc10{"acc10", KeyType::ed25519}; + jtx::Account const acc11{"acc11", KeyType::ed25519}; + jtx::Account const acc12{"acc12", KeyType::ed25519}; + jtx::Account const acc13{"acc13", KeyType::ed25519}; + jtx::Account const acc14{"acc14", KeyType::ed25519}; + jtx::Account const acc15{"acc15", KeyType::ed25519}; + jtx::Account const acc16{"acc16", KeyType::ed25519}; + jtx::Account const acc17{"acc17", KeyType::ed25519}; + jtx::Account const acc18{"acc18", KeyType::ed25519}; + jtx::Account const acc19{"acc19", KeyType::ed25519}; + jtx::Account const acc20{"acc20", KeyType::ed25519}; + jtx::Account const acc21{"acc21", KeyType::ed25519}; + jtx::Account const acc22{"acc22", KeyType::ed25519}; + jtx::Account const acc23{"acc23", KeyType::ed25519}; + jtx::Account const acc24{"acc24", KeyType::ed25519}; + jtx::Account const acc25{"acc25", KeyType::ed25519}; + jtx::Account const acc26{"acc26", KeyType::ed25519}; + jtx::Account const acc27{"acc27", KeyType::ed25519}; + jtx::Account const acc28{"acc28", KeyType::ed25519}; + jtx::Account const acc29{"acc29", KeyType::ed25519}; + jtx::Account const acc30{"acc30", KeyType::ed25519}; + jtx::Account const acc31{"acc31", KeyType::ed25519}; + jtx::Account const acc32{"acc32", KeyType::ed25519}; + jtx::Account const acc33{"acc33", KeyType::ed25519}; public: void @@ -159,22 +183,30 @@ class MultiSign_test : public beast::unit_test::suite {spook, 1}}), ter(temBAD_QUORUM)); - // Make a signer list that's too big. Should fail. + // clang-format off + // Make a signer list that's too big. Should fail. (Even with + // ExpandedSignerList) Account const spare("spare", KeyType::secp256k1); env(signers( alice, 1, - {{bogie, 1}, - {demon, 1}, - {ghost, 1}, - {haunt, 1}, - {jinni, 1}, - {phase, 1}, - {shade, 1}, - {spook, 1}, - {spare, 1}}), + features[featureExpandedSignerList] + ? 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}, + {acc13, 1}, {acc14, 1}, {acc15, 1}, + {acc16, 1}, {acc17, 1}, {acc18, 1}, + {acc19, 1}, {acc20, 1}, {acc21, 1}, + {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}}), ter(temMALFORMED)); - + // clang-format on env.close(); env.require(owners(alice, 0)); } @@ -1149,20 +1181,56 @@ class MultiSign_test : public beast::unit_test::suite "fails local checks: Invalid Signers array size."); } { - // Multisign 9 times should fail. + // Multisign 9 (!ExpandedSignerList) | 33 (ExpandedSignerList) times + // should fail. JTx tx = env.jt( noop(alice), fee(2 * baseFee), - msig( - bogie, - bogie, - bogie, - bogie, - bogie, - bogie, - bogie, - bogie, - bogie)); + + 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)); STTx local = *(tx.stx); auto const info = submitSTTx(local); BEAST_EXPECT( @@ -1517,6 +1585,82 @@ class MultiSign_test : public beast::unit_test::suite BEAST_EXPECT(env.seq(alice) == aliceSeq); } + void + test_signersWithTags(FeatureBitset features) + { + if (!features[featureExpandedSignerList]) + return; + + testcase("Signers With Tags"); + + using namespace jtx; + Env env{*this, features}; + Account const alice{"alice", KeyType::ed25519}; + env.fund(XRP(1000), alice); + env.close(); + uint8_t tag1[] = {0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, + 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, + 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, + 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08}; + + uint8_t tag2[] = + "hello world some ascii 32b long"; // including 1 byte for NUL + + uint256 bogie_tag = ripple::base_uint<256>::fromVoid(tag1); + uint256 demon_tag = ripple::base_uint<256>::fromVoid(tag2); + + // 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)); + + // This should work. + auto const baseFee = env.current()->fees().base; + std::uint32_t aliceSeq = env.seq(alice); + env(noop(alice), msig(bogie, demon), fee(3 * baseFee)); + env.close(); + BEAST_EXPECT(env.seq(alice) == aliceSeq + 1); + + // Either signer alone should work. + aliceSeq = env.seq(alice); + env(noop(alice), msig(bogie), fee(2 * baseFee)); + env.close(); + BEAST_EXPECT(env.seq(alice) == aliceSeq + 1); + + aliceSeq = env.seq(alice); + env(noop(alice), msig(demon), fee(2 * baseFee)); + env.close(); + BEAST_EXPECT(env.seq(alice) == aliceSeq + 1); + + // Duplicate signers should fail. + aliceSeq = env.seq(alice); + env(noop(alice), msig(demon, demon), fee(3 * baseFee), ter(temINVALID)); + env.close(); + BEAST_EXPECT(env.seq(alice) == aliceSeq); + + // A non-signer should fail. + aliceSeq = env.seq(alice); + env(noop(alice), + msig(bogie, spook), + fee(3 * baseFee), + ter(tefBAD_SIGNATURE)); + env.close(); + BEAST_EXPECT(env.seq(alice) == aliceSeq); + + // Don't meet the quorum. Should fail. + env(signers(alice, 2, {{bogie, 1}, {demon, 1}})); + aliceSeq = env.seq(alice); + env(noop(alice), msig(bogie), fee(2 * baseFee), ter(tefBAD_QUORUM)); + env.close(); + BEAST_EXPECT(env.seq(alice) == aliceSeq); + + // Meet the quorum. Should succeed. + aliceSeq = env.seq(alice); + env(noop(alice), msig(bogie, demon), fee(3 * baseFee)); + env.close(); + BEAST_EXPECT(env.seq(alice) == aliceSeq + 1); + } + void testAll(FeatureBitset features) { @@ -1537,6 +1681,7 @@ class MultiSign_test : public beast::unit_test::suite test_multisigningMultisigner(features); test_signForHash(features); test_signersWithTickets(features); + test_signersWithTags(features); } void @@ -1545,10 +1690,13 @@ class MultiSign_test : public beast::unit_test::suite using namespace jtx; auto const all = supported_amendments(); - // The reserve required on a signer list changes based on. - // featureMultiSignReserve. Test both with and without. - testAll(all - featureMultiSignReserve); - testAll(all | featureMultiSignReserve); + // 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); test_amendmentTransition(); } }; diff --git a/src/test/jtx/impl/multisign.cpp b/src/test/jtx/impl/multisign.cpp index 129f3070145..1e1f5141798 100644 --- a/src/test/jtx/impl/multisign.cpp +++ b/src/test/jtx/impl/multisign.cpp @@ -22,6 +22,8 @@ #include #include #include +#include +#include #include #include @@ -46,6 +48,8 @@ signers( auto& je = ja[i][sfSignerEntry.getJsonName()]; je[jss::Account] = e.account.human(); je[sfSignerWeight.getJsonName()] = e.weight; + if (e.tag) + je[sfWalletLocator.getJsonName()] = to_string(*e.tag); } return jv; } diff --git a/src/test/jtx/multisign.h b/src/test/jtx/multisign.h index 91a110352cd..ab9996e415d 100644 --- a/src/test/jtx/multisign.h +++ b/src/test/jtx/multisign.h @@ -21,6 +21,7 @@ #define RIPPLE_TEST_JTX_MULTISIGN_H_INCLUDED #include +#include #include #include #include @@ -35,9 +36,13 @@ struct signer { std::uint32_t weight; Account account; + std::optional tag; - signer(Account account_, std::uint32_t weight_ = 1) - : weight(weight_), account(std::move(account_)) + signer( + Account account_, + std::uint32_t weight_ = 1, + std::optional tag_ = std::nullopt) + : weight(weight_), account(std::move(account_)), tag(std::move(tag_)) { } }; diff --git a/src/test/protocol/STTx_test.cpp b/src/test/protocol/STTx_test.cpp index 5d24efcfcd4..4ef30fb7a74 100644 --- a/src/test/protocol/STTx_test.cpp +++ b/src/test/protocol/STTx_test.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -27,7 +28,6 @@ #include #include #include - #include #include @@ -1591,8 +1591,10 @@ class STTx_test : public beast::unit_test::suite }); j.sign(keypair.first, keypair.second); + Rules defaultRules{{}}; + unexpected( - !j.checkSign(STTx::RequireFullyCanonicalSig::yes), + !j.checkSign(STTx::RequireFullyCanonicalSig::yes, defaultRules), "Transaction fails signature test"); Serializer rawTxn;