Compare commits

..

3 Commits

Author SHA1 Message Date
RichardAH
8cec97d57c Merge branch 'dev' into fix-manifest-cache-invalidation 2025-12-17 09:45:51 +10:00
Niq Dudfield
f818174b8d Merge branch 'dev' into fix-manifest-cache-invalidation 2025-12-16 14:33:00 +07:00
Nicholas Dudfield
684c2ac108 fix: increment manifest sequence for client code cache invalidation
OverlayImpl::getManifestsMessage() caches manifest messages and only rebuilds
them when ManifestCache::sequence() changes. When accepting a new manifest,
the function returned early without incrementing seq_, causing the cache to
never invalidate. This meant peers were exchanging stale lists.

Now seq_++ is called for both new manifests and updates, ensuring the overlay
layer detects changes and sends complete validator lists to connecting peers.

Closes #629
2025-11-21 10:22:25 +07:00
13 changed files with 246 additions and 1462 deletions

View File

@@ -134,17 +134,10 @@ runs:
- name: Export custom recipes
shell: bash
run: |
# Export snappy if not already exported
conan list snappy/1.1.10@xahaud/stable 2>/dev/null | (grep -q "not found" && exit 1 || exit 0) || \
conan export external/snappy --version 1.1.10 --user xahaud --channel stable
# Export soci if not already exported
conan list soci/4.0.3@xahaud/stable 2>/dev/null | (grep -q "not found" && exit 1 || exit 0) || \
conan export external/soci --version 4.0.3 --user xahaud --channel stable
# Export wasmedge if not already exported
conan list wasmedge/0.11.2@xahaud/stable 2>/dev/null | (grep -q "not found" && exit 1 || exit 0) || \
conan export external/wasmedge --version 0.11.2 --user xahaud --channel stable
conan export external/snappy --version 1.1.10 --user xahaud --channel stable
conan export external/soci --version 4.0.3 --user xahaud --channel stable
conan export external/wasmedge --version 0.11.2 --user xahaud --channel stable
- name: Install dependencies
shell: bash
env:

View File

@@ -43,22 +43,14 @@ jobs:
# To isolate environments for each Runner, instead of installing globally with brew,
# use mise to isolate environments for each Runner directory.
- name: Setup toolchain (mise)
uses: jdx/mise-action@v3.6.1
uses: jdx/mise-action@v2
with:
cache: false
install: true
mise_toml: |
[tools]
cmake = "3.23.1"
python = "3.12"
pipx = "latest"
conan = "2"
ninja = "latest"
ccache = "latest"
- name: Install tools via mise
run: |
mise install
mise use cmake@3.23.1 python@3.12 pipx@latest conan@2 ninja@latest ccache@latest
mise reshim
echo "$HOME/.local/share/mise/shims" >> "$GITHUB_PATH"

View File

@@ -289,40 +289,8 @@ Transactor::calculateBaseFee(ReadView const& view, STTx const& tx)
// Each signer adds one more baseFee to the minimum required fee
// for the transaction.
std::size_t signerCount = 0;
if (tx.isFieldPresent(sfSigners))
{
// Define recursive lambda to count all leaf signers
std::function<std::size_t(STArray const&)> countSigners;
countSigners = [&](STArray const& signers) -> std::size_t {
std::size_t count = 0;
for (auto const& signer : signers)
{
if (signer.isFieldPresent(sfSigners))
{
// This is a nested signer - recursively count its signers
count += countSigners(signer.getFieldArray(sfSigners));
}
else
{
// This is a leaf signer (one who actually signs)
// Count it only if it has signing fields (not just a
// placeholder)
if (signer.isFieldPresent(sfSigningPubKey) &&
signer.isFieldPresent(sfTxnSignature))
{
count += 1;
}
}
}
return count;
};
signerCount = countSigners(tx.getFieldArray(sfSigners));
}
std::size_t const signerCount =
tx.isFieldPresent(sfSigners) ? tx.getFieldArray(sfSigners).size() : 0;
XRPAmount hookExecutionFee{0};
uint64_t burden{1};
@@ -955,282 +923,157 @@ NotTEC
Transactor::checkMultiSign(PreclaimContext const& ctx)
{
auto const id = ctx.tx.getAccountID(sfAccount);
// Set max depth based on feature flag
bool const allowNested = ctx.view.rules().enabled(featureNestedMultiSign);
int const maxDepth = allowNested ? 4 : 1;
// Define recursive lambda for checking signers at any depth
// ancestors tracks the signing chain to detect cycles
std::function<NotTEC(
AccountID const&, STArray const&, int, std::set<AccountID>)>
validateSigners;
validateSigners = [&](AccountID const& acc,
STArray const& signers,
int depth,
std::set<AccountID> ancestors) -> NotTEC {
// Cycle detection: if we're already validating this account up the
// chain it cannot contribute - but this isn't an error, just
// unavailable weight
if (ancestors.count(acc))
{
JLOG(ctx.j.trace())
<< "checkMultiSign: Cyclic signer detected: " << acc;
return tesSUCCESS;
}
// Check depth limit
if (depth > maxDepth)
{
if (allowNested)
{
JLOG(ctx.j.trace())
<< "checkMultiSign: Multi-signing depth limit exceeded.";
return tefBAD_SIGNATURE;
}
JLOG(ctx.j.warn())
<< "checkMultiSign: Nested multisigning disabled.";
return temMALFORMED;
}
ancestors.insert(acc);
// Get the SignerList for the account we're validating signers for
std::shared_ptr<STLedgerEntry const> sleAllowedSigners =
ctx.view.read(keylet::signers(acc));
if (!sleAllowedSigners)
{
JLOG(ctx.j.trace()) << "checkMultiSign: Account " << acc
<< " not set up for multi-signing.";
return tefNOT_MULTI_SIGNING;
}
uint32_t const quorum = sleAllowedSigners->getFieldU32(sfSignerQuorum);
uint32_t sum{0};
auto allowedSigners =
SignerEntries::deserialize(*sleAllowedSigners, ctx.j, "ledger");
if (!allowedSigners)
return allowedSigners.error();
// Build lookup map for O(1) signer validation and weight retrieval
std::map<AccountID, uint16_t> signerWeights;
uint32_t totalWeight{0}, cyclicWeight{0};
for (auto const& entry : *allowedSigners)
{
signerWeights[entry.account] = entry.weight;
totalWeight += entry.weight;
if (ancestors.count(entry.account))
cyclicWeight += entry.weight;
}
// Walk the signers array, validating each signer
// Signers must be in strict ascending order for consensus
std::optional<AccountID> prevSigner;
for (auto const& signerEntry : signers)
{
AccountID const signer = signerEntry.getAccountID(sfAccount);
bool const isNested = signerEntry.isFieldPresent(sfSigners);
// Enforce strict ascending order (required for consensus)
if (prevSigner && signer <= *prevSigner)
{
JLOG(ctx.j.trace())
<< "checkMultiSign: Signers not in strict ascending order: "
<< signer << " <= " << *prevSigner;
return temMALFORMED;
}
prevSigner = signer;
// Skip cyclic signers - they cannot contribute at this level
if (ancestors.count(signer))
{
JLOG(ctx.j.trace())
<< "checkMultiSign: Skipping cyclic signer: " << signer;
continue;
}
// Lookup signer in authorized set
auto const weightIt = signerWeights.find(signer);
if (weightIt == signerWeights.end())
{
JLOG(ctx.j.trace())
<< "checkMultiSign: Invalid signer " << signer
<< " not in signer list for " << acc;
return tefBAD_SIGNATURE;
}
uint16_t const weight = weightIt->second;
// Check if this signer has nested signers (delegation)
if (isNested)
{
// This is a nested multi-signer that delegates to sub-signers
if (signerEntry.isFieldPresent(sfSigningPubKey) ||
signerEntry.isFieldPresent(sfTxnSignature))
{
JLOG(ctx.j.trace()) << "checkMultiSign: Signer " << signer
<< " cannot have both nested signers "
"and signature fields.";
return tefBAD_SIGNATURE;
}
// Recursively validate the nested signers against signer's
// signer list
STArray const& nestedSigners =
signerEntry.getFieldArray(sfSigners);
NotTEC result = validateSigners(
signer, nestedSigners, depth + 1, ancestors);
if (!isTesSuccess(result))
return result;
// Nested signers met their quorum - add this signer's weight
sum += weight;
JLOG(ctx.j.trace())
<< "checkMultiSign: Nested signer " << signer
<< " validated, weight=" << weight << ", depth=" << depth
<< ", sum=" << sum << "/" << quorum;
}
else
{
// This is a leaf signer - validate signature
if (!signerEntry.isFieldPresent(sfSigningPubKey) ||
!signerEntry.isFieldPresent(sfTxnSignature))
{
JLOG(ctx.j.trace())
<< "checkMultiSign: Leaf signer " << signer
<< " must have SigningPubKey and TxnSignature.";
return tefBAD_SIGNATURE;
}
auto const spk = signerEntry.getFieldVL(sfSigningPubKey);
if (!publicKeyType(makeSlice(spk)))
{
JLOG(ctx.j.trace())
<< "checkMultiSign: Unknown public key type for signer "
<< signer;
return tefBAD_SIGNATURE;
}
AccountID const signingAcctIDFromPubKey =
calcAccountID(PublicKey(makeSlice(spk)));
auto sleTxSignerRoot = ctx.view.read(keylet::account(signer));
if (signingAcctIDFromPubKey == signer)
{
if (sleTxSignerRoot)
{
std::uint32_t const signerAccountFlags =
sleTxSignerRoot->getFieldU32(sfFlags);
if (signerAccountFlags & lsfDisableMaster)
{
JLOG(ctx.j.trace())
<< "checkMultiSign: Signer " << signer
<< " has lsfDisableMaster set.";
return tefMASTER_DISABLED;
}
}
}
else
{
if (!sleTxSignerRoot)
{
JLOG(ctx.j.trace())
<< "checkMultiSign: Non-phantom signer " << signer
<< " lacks account root.";
return tefBAD_SIGNATURE;
}
if (!sleTxSignerRoot->isFieldPresent(sfRegularKey))
{
JLOG(ctx.j.trace()) << "checkMultiSign: Signer "
<< signer << " lacks RegularKey.";
return tefBAD_SIGNATURE;
}
if (signingAcctIDFromPubKey !=
sleTxSignerRoot->getAccountID(sfRegularKey))
{
JLOG(ctx.j.trace())
<< "checkMultiSign: Signer " << signer
<< " pubkey doesn't match RegularKey.";
return tefBAD_SIGNATURE;
}
}
// Valid leaf signer - add their weight
sum += weight;
JLOG(ctx.j.trace())
<< "checkMultiSign: Leaf signer " << signer
<< " validated, weight=" << weight << ", depth=" << depth
<< ", sum=" << sum << "/" << quorum;
}
}
// Calculate effective quorum, relaxing for cyclic lockout scenarios
// Sanity check: cyclicWeight must not exceed totalWeight (underflow
// guard)
if (cyclicWeight > totalWeight)
{
JLOG(ctx.j.error()) << "checkMultiSign: Invariant violation for "
<< acc << ": cyclicWeight (" << cyclicWeight
<< ") > totalWeight (" << totalWeight << ")";
return tefINTERNAL;
}
uint32_t effectiveQuorum = quorum;
uint32_t const maxAchievable = totalWeight - cyclicWeight;
if (cyclicWeight > 0 && maxAchievable < quorum)
{
JLOG(ctx.j.warn())
<< "checkMultiSign: Cyclic lockout detected for " << acc
<< ": relaxing quorum from " << quorum << " to "
<< maxAchievable << " (total=" << totalWeight
<< ", cyclic=" << cyclicWeight << ")";
effectiveQuorum = maxAchievable;
}
// Sanity check: effectiveQuorum of 0 means all signers are cyclic -
// this is an irrecoverable misconfiguration
if (effectiveQuorum == 0)
{
JLOG(ctx.j.warn()) << "checkMultiSign: All signers for " << acc
<< " are cyclic - no valid signing path exists.";
return tefBAD_QUORUM;
}
// Check if accumulated weight meets required quorum
if (sum < effectiveQuorum)
{
JLOG(ctx.j.trace()) << "checkMultiSign: Quorum not met for " << acc
<< " at depth " << depth << " (sum=" << sum
<< ", required=" << effectiveQuorum << ")";
return tefBAD_QUORUM;
}
return tesSUCCESS;
};
STArray const& entries(ctx.tx.getFieldArray(sfSigners));
// Initial call with empty ancestor set - the function inserts acc after
// cycle check
NotTEC result = validateSigners(id, entries, 1, {});
if (!isTesSuccess(result))
// Get mTxnAccountID's SignerList and Quorum.
std::shared_ptr<STLedgerEntry const> sleAccountSigners =
ctx.view.read(keylet::signers(id));
// If the signer list doesn't exist the account is not multi-signing.
if (!sleAccountSigners)
{
JLOG(ctx.j.trace())
<< "checkMultiSign: Validation failed with " << transToken(result);
return result;
<< "applyTransaction: Invalid: Not a multi-signing account.";
return tefNOT_MULTI_SIGNING;
}
// We have plans to support multiple SignerLists in the future. The
// presence and defaulted value of the SignerListID field will enable that.
assert(sleAccountSigners->isFieldPresent(sfSignerListID));
assert(sleAccountSigners->getFieldU32(sfSignerListID) == 0);
auto accountSigners =
SignerEntries::deserialize(*sleAccountSigners, ctx.j, "ledger");
if (!accountSigners)
return accountSigners.error();
// Get the array of transaction signers.
STArray const& txSigners(ctx.tx.getFieldArray(sfSigners));
// Walk the accountSigners performing a variety of checks and see if
// the quorum is met.
// Both the multiSigners and accountSigners are sorted by account. So
// matching multi-signers to account signers should be a simple
// linear walk. *All* signers must be valid or the transaction fails.
std::uint32_t weightSum = 0;
auto iter = accountSigners->begin();
for (auto const& txSigner : txSigners)
{
AccountID const txSignerAcctID = txSigner.getAccountID(sfAccount);
// Attempt to match the SignerEntry with a Signer;
while (iter->account < txSignerAcctID)
{
if (++iter == accountSigners->end())
{
JLOG(ctx.j.trace())
<< "applyTransaction: Invalid SigningAccount.Account.";
return tefBAD_SIGNATURE;
}
}
if (iter->account != txSignerAcctID)
{
// The SigningAccount is not in the SignerEntries.
JLOG(ctx.j.trace())
<< "applyTransaction: Invalid SigningAccount.Account.";
return tefBAD_SIGNATURE;
}
// We found the SigningAccount in the list of valid signers. Now we
// need to compute the accountID that is associated with the signer's
// public key.
auto const spk = txSigner.getFieldVL(sfSigningPubKey);
if (!publicKeyType(makeSlice(spk)))
{
JLOG(ctx.j.trace())
<< "checkMultiSign: signing public key type is unknown";
return tefBAD_SIGNATURE;
}
AccountID const signingAcctIDFromPubKey =
calcAccountID(PublicKey(makeSlice(spk)));
// Verify that the signingAcctID and the signingAcctIDFromPubKey
// belong together. Here is are the rules:
//
// 1. "Phantom account": an account that is not in the ledger
// A. If signingAcctID == signingAcctIDFromPubKey and the
// signingAcctID is not in the ledger then we have a phantom
// account.
// B. Phantom accounts are always allowed as multi-signers.
//
// 2. "Master Key"
// A. signingAcctID == signingAcctIDFromPubKey, and signingAcctID
// is in the ledger.
// B. If the signingAcctID in the ledger does not have the
// asfDisableMaster flag set, then the signature is allowed.
//
// 3. "Regular Key"
// A. signingAcctID != signingAcctIDFromPubKey, and signingAcctID
// is in the ledger.
// B. If signingAcctIDFromPubKey == signingAcctID.RegularKey (from
// ledger) then the signature is allowed.
//
// No other signatures are allowed. (January 2015)
// In any of these cases we need to know whether the account is in
// the ledger. Determine that now.
auto sleTxSignerRoot = ctx.view.read(keylet::account(txSignerAcctID));
if (signingAcctIDFromPubKey == txSignerAcctID)
{
// Either Phantom or Master. Phantoms automatically pass.
if (sleTxSignerRoot)
{
// Master Key. Account may not have asfDisableMaster set.
std::uint32_t const signerAccountFlags =
sleTxSignerRoot->getFieldU32(sfFlags);
if (signerAccountFlags & lsfDisableMaster)
{
JLOG(ctx.j.trace())
<< "applyTransaction: Signer:Account lsfDisableMaster.";
return tefMASTER_DISABLED;
}
}
}
else
{
// May be a Regular Key. Let's find out.
// Public key must hash to the account's regular key.
if (!sleTxSignerRoot)
{
JLOG(ctx.j.trace()) << "applyTransaction: Non-phantom signer "
"lacks account root.";
return tefBAD_SIGNATURE;
}
if (!sleTxSignerRoot->isFieldPresent(sfRegularKey))
{
JLOG(ctx.j.trace())
<< "applyTransaction: Account lacks RegularKey.";
return tefBAD_SIGNATURE;
}
if (signingAcctIDFromPubKey !=
sleTxSignerRoot->getAccountID(sfRegularKey))
{
JLOG(ctx.j.trace())
<< "applyTransaction: Account doesn't match RegularKey.";
return tefBAD_SIGNATURE;
}
}
// The signer is legitimate. Add their weight toward the quorum.
weightSum += iter->weight;
}
// Cannot perform transaction if quorum is not met.
if (weightSum < sleAccountSigners->getFieldU32(sfSignerQuorum))
{
JLOG(ctx.j.trace())
<< "applyTransaction: Signers failed to meet quorum.";
return tefBAD_QUORUM;
}
// Met the quorum. Continue.
return tesSUCCESS;
}

View File

@@ -484,61 +484,44 @@ OverlayImpl::start()
m_peerFinder->setConfig(config);
m_peerFinder->start();
auto addIps = [this](std::vector<std::string> ips, bool fixed) {
auto addIps = [&](std::vector<std::string> bootstrapIps) -> void {
beast::Journal const& j = app_.journal("Overlay");
for (auto& ip : ips)
for (auto& ip : bootstrapIps)
{
std::size_t pos = ip.find('#');
if (pos != std::string::npos)
ip.erase(pos);
JLOG(j.trace())
<< "Found " << (fixed ? "fixed" : "bootstrap") << " IP: " << ip;
JLOG(j.trace()) << "Found boostrap IP: " << ip;
}
m_resolver.resolve(
ips,
[this, fixed](
std::string const& name,
bootstrapIps,
[&](std::string const& name,
std::vector<beast::IP::Endpoint> const& addresses) {
std::vector<std::string> ips;
ips.reserve(addresses.size());
beast::Journal const& j = app_.journal("Overlay");
std::string const base("config: ");
std::vector<beast::IP::Endpoint> eps;
eps.reserve(addresses.size());
for (auto const& addr : addresses)
{
auto ep = addr.port() == 0 ? addr.at_port(DEFAULT_PEER_PORT)
: addr;
JLOG(j.trace())
<< "Parsed " << (fixed ? "fixed" : "bootstrap")
<< " IP: " << ep;
eps.push_back(ep);
std::string addrStr = addr.port() == 0
? to_string(addr.at_port(DEFAULT_PEER_PORT))
: to_string(addr);
JLOG(j.trace()) << "Parsed boostrap IP: " << addrStr;
ips.push_back(addrStr);
}
if (eps.empty())
return;
if (fixed)
{
m_peerFinder->addFixedPeer(base + name, eps);
}
else
{
std::vector<std::string> strs;
strs.reserve(eps.size());
for (auto const& ep : eps)
strs.push_back(to_string(ep));
m_peerFinder->addFallbackStrings(base + name, strs);
}
std::string const base("config: ");
if (!ips.empty())
m_peerFinder->addFallbackStrings(base + name, ips);
});
};
if (!app_.config().IPS.empty())
addIps(app_.config().IPS, false);
addIps(app_.config().IPS);
if (!app_.config().IPS_FIXED.empty())
addIps(app_.config().IPS_FIXED, true);
addIps(app_.config().IPS_FIXED);
auto const timer = std::make_shared<Timer>(*this);
std::lock_guard lock(mutex_);

View File

@@ -74,7 +74,7 @@ namespace detail {
// Feature.cpp. Because it's only used to reserve storage, and determine how
// large to make the FeatureBitset, it MAY be larger. It MUST NOT be less than
// the actual number of amendments. A LogicError on startup will verify this.
static constexpr std::size_t numFeatures = 91;
static constexpr std::size_t numFeatures = 90;
/** Amendments that this server supports and the default voting behavior.
Whether they are enabled depends on the Rules defined in the validated
@@ -378,7 +378,6 @@ extern uint256 const fixInvalidTxFlags;
extern uint256 const featureExtendedHookState;
extern uint256 const fixCronStacking;
extern uint256 const fixHookAPI20251128;
extern uint256 const featureNestedMultiSign;
} // namespace ripple
#endif

View File

@@ -484,7 +484,6 @@ REGISTER_FIX (fixInvalidTxFlags, Supported::yes, VoteBehavior::De
REGISTER_FEATURE(ExtendedHookState, Supported::yes, VoteBehavior::DefaultNo);
REGISTER_FIX (fixCronStacking, Supported::yes, VoteBehavior::DefaultYes);
REGISTER_FIX (fixHookAPI20251128, Supported::yes, VoteBehavior::DefaultYes);
REGISTER_FEATURE(NestedMultiSign, Supported::yes, VoteBehavior::DefaultNo);
// The following amendments are obsolete, but must remain supported
// because they could potentially get enabled.

View File

@@ -44,9 +44,8 @@ InnerObjectFormats::InnerObjectFormats()
sfSigner.getCode(),
{
{sfAccount, soeREQUIRED},
{sfSigningPubKey, soeOPTIONAL},
{sfTxnSignature, soeOPTIONAL},
{sfSigners, soeOPTIONAL},
{sfSigningPubKey, soeREQUIRED},
{sfTxnSignature, soeREQUIRED},
});
add(sfMajority.jsonName.c_str(),

View File

@@ -369,146 +369,64 @@ STTx::checkMultiSign(
bool const fullyCanonical = (getFlags() & tfFullyCanonicalSig) ||
(requireCanonicalSig == RequireFullyCanonicalSig::yes);
// Signers must be in sorted order by AccountID.
AccountID lastAccountID(beast::zero);
bool const isWildcardNetwork =
isFieldPresent(sfNetworkID) && getFieldU32(sfNetworkID) == 65535;
// Set max depth based on feature flag
int const maxDepth = rules.enabled(featureNestedMultiSign) ? 4 : 1;
for (auto const& signer : signers)
{
auto const accountID = signer.getAccountID(sfAccount);
// Define recursive lambda for checking signatures at any depth
std::function<Expected<void, std::string>(
STArray const&, AccountID const&, int)>
checkSignersArray;
// The account owner may not multisign for themselves.
if (accountID == txnAccountID)
return Unexpected("Invalid multisigner.");
checkSignersArray = [&](STArray const& signersArray,
AccountID const& parentAccountID,
int depth) -> Expected<void, std::string> {
// Check depth limit
if (depth > maxDepth)
// No duplicate signers allowed.
if (lastAccountID == accountID)
return Unexpected("Duplicate Signers not allowed.");
// Accounts must be in order by account ID. No duplicates allowed.
if (lastAccountID > accountID)
return Unexpected("Unsorted Signers array.");
// The next signature must be greater than this one.
lastAccountID = accountID;
// Verify the signature.
bool validSig = false;
try
{
std::cout << "Multi-signing depth limit exceeded.\n";
return Unexpected("Multi-signing depth limit exceeded.");
}
Serializer s = dataStart;
finishMultiSigningData(accountID, s);
// There are well known bounds that the number of signers must be
// within.
if (signersArray.size() < minMultiSigners ||
signersArray.size() > maxMultiSigners(&rules))
{
std::cout << "Invalid Signers array size.\n";
return Unexpected("Invalid Signers array size.");
}
auto spk = signer.getFieldVL(sfSigningPubKey);
// Signers must be in sorted order by AccountID.
AccountID lastAccountID(beast::zero);
for (auto const& signer : signersArray)
{
auto const accountID = signer.getAccountID(sfAccount);
// The account owner may not multisign for themselves.
if (accountID == txnAccountID)
if (publicKeyType(makeSlice(spk)))
{
std::cout << "Invalid multisigner.\n";
return Unexpected("Invalid multisigner.");
}
Blob const signature = signer.getFieldVL(sfTxnSignature);
// No duplicate signers allowed.
if (lastAccountID == accountID)
{
std::cout << "Duplicate Signers not allowed.\n";
return Unexpected("Duplicate Signers not allowed.");
}
// Accounts must be in order by account ID. No duplicates allowed.
if (lastAccountID > accountID)
{
std::cout << "Unsorted Signers array.\n";
return Unexpected("Unsorted Signers array.");
}
// The next signature must be greater than this one.
lastAccountID = accountID;
// Check if this signer has nested signers
if (signer.isFieldPresent(sfSigners))
{
// This is a nested multi-signer
// Ensure it doesn't also have signature fields
if (signer.isFieldPresent(sfSigningPubKey) ||
signer.isFieldPresent(sfTxnSignature))
{
std::cout << "Signer cannot have both nested signers and "
"signature "
"fields.\n";
return Unexpected(
"Signer cannot have both nested signers and signature "
"fields.");
}
// Recursively check nested signers
STArray const& nestedSigners = signer.getFieldArray(sfSigners);
auto result =
checkSignersArray(nestedSigners, accountID, depth + 1);
if (!result)
return result;
}
else
{
// This is a leaf node - must have signature
if (!signer.isFieldPresent(sfSigningPubKey) ||
!signer.isFieldPresent(sfTxnSignature))
{
std::cout << "Leaf signer must have SigningPubKey and "
"TxnSignature.\n";
return Unexpected(
"Leaf signer must have SigningPubKey and "
"TxnSignature.");
}
// Verify the signature
bool validSig = false;
try
{
Serializer s = dataStart;
finishMultiSigningData(accountID, s);
auto spk = signer.getFieldVL(sfSigningPubKey);
if (publicKeyType(makeSlice(spk)))
{
Blob const signature =
signer.getFieldVL(sfTxnSignature);
// wildcard network gets a free pass
validSig = isWildcardNetwork ||
verify(PublicKey(makeSlice(spk)),
s.slice(),
makeSlice(signature),
fullyCanonical);
}
}
catch (std::exception const&)
{
// We assume any problem lies with the signature.
validSig = false;
}
if (!validSig)
{
std::cout << std::string("Invalid signature on account ") +
toBase58(accountID) + ".\n";
return Unexpected(
std::string("Invalid signature on account ") +
toBase58(accountID) + ".");
}
// wildcard network gets a free pass
validSig = isWildcardNetwork ||
verify(PublicKey(makeSlice(spk)),
s.slice(),
makeSlice(signature),
fullyCanonical);
}
}
return {};
};
// Start the recursive check at depth 1
return checkSignersArray(signers, txnAccountID, 1);
catch (std::exception const&)
{
// We assume any problem lies with the signature.
validSig = false;
}
if (!validSig)
return Unexpected(
std::string("Invalid signature on account ") +
toBase58(accountID) + ".");
}
// All signatures verified.
return {};
}
//------------------------------------------------------------------------------

View File

@@ -1183,21 +1183,12 @@ transactionSubmitMultiSigned(
// The Signers array may only contain Signer objects.
if (std::find_if_not(
signers.begin(), signers.end(), [](STObject const& obj) {
if (obj.getCount() != 4 || !obj.isFieldPresent(sfAccount))
return false;
// leaf signer
if (obj.isFieldPresent(sfSigningPubKey) &&
obj.isFieldPresent(sfTxnSignature) &&
!obj.isFieldPresent(sfSigners))
return true;
// nested signer
if (!obj.isFieldPresent(sfSigningPubKey) &&
!obj.isFieldPresent(sfTxnSignature) &&
obj.isFieldPresent(sfSigners))
return true;
return false;
return (
// A Signer object always contains these fields and no
// others.
obj.isFieldPresent(sfAccount) &&
obj.isFieldPresent(sfSigningPubKey) &&
obj.isFieldPresent(sfTxnSignature) && obj.getCount() == 3);
}) != signers.end())
{
return RPC::make_param_error(

View File

@@ -1659,800 +1659,6 @@ public:
BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
}
void
test_nestedMultiSign(FeatureBitset features)
{
testcase("Nested MultiSign");
#define STRINGIFY(x) #x
#define TOSTRING(x) STRINGIFY(x)
#define LINE_TO_HEX_STRING \
[]() -> std::string { \
const char* line = TOSTRING(__LINE__); \
int len = 0; \
while (line[len]) \
len++; \
std::string result; \
if (len % 2 == 1) \
{ \
result += (char)(0x00 * 16 + (line[0] - '0')); \
line++; \
} \
for (int i = 0; line[i]; i += 2) \
{ \
result += (char)((line[i] - '0') * 16 + (line[i + 1] - '0')); \
} \
return result; \
}()
#define M(m) memo(m, "", "")
#define L() memo(LINE_TO_HEX_STRING, "", "")
using namespace jtx;
Env env{*this, envconfig(), features};
// Env env{*this, envconfig(), features, nullptr,
// beast::severities::kTrace};
Account const alice{"alice", KeyType::secp256k1};
Account const becky{"becky", KeyType::ed25519};
Account const cheri{"cheri", KeyType::secp256k1};
Account const daria{"daria", KeyType::ed25519};
Account const edgar{"edgar", KeyType::secp256k1};
Account const fiona{"fiona", KeyType::ed25519};
Account const grace{"grace", KeyType::secp256k1};
Account const henry{"henry", KeyType::ed25519};
Account const f1{"f1", KeyType::ed25519};
Account const f2{"f2", KeyType::ed25519};
Account const f3{"f3", KeyType::ed25519};
env.fund(
XRP(1000),
alice,
becky,
cheri,
daria,
edgar,
fiona,
grace,
henry,
f1,
f2,
f3,
phase,
jinni,
acc10,
acc11,
acc12);
env.close();
auto const baseFee = env.current()->fees().base;
if (!features[featureNestedMultiSign])
{
// When feature is disabled, nested signing should fail
env(signers(f1, 1, {{f2, 1}}));
env(signers(f2, 1, {{f3, 1}}));
env.close();
std::uint32_t f1Seq = env.seq(f1);
env(noop(f1),
msig({msigner(f2, msigner(f3))}),
L(),
fee(3 * baseFee),
ter(temINVALID));
env.close();
BEAST_EXPECT(env.seq(f1) == f1Seq);
return;
}
// Test Case 1: Basic 2-level nested signing with quorum
{
// Set up signer lists with quorum requirements
env(signers(becky, 2, {{bogie, 1}, {demon, 1}, {ghost, 1}}));
env(signers(cheri, 3, {{haunt, 2}, {jinni, 2}}));
env.close();
// Alice requires quorum of 3 with weighted signers
env(signers(alice, 3, {{becky, 2}, {cheri, 2}, {daria, 1}}));
env.close();
// Test 1a: becky alone (weight 2) doesn't meet alice's quorum
std::uint32_t aliceSeq = env.seq(alice);
env(noop(alice),
msig({msigner(becky, msigner(bogie), msigner(demon))}),
L(),
fee(4 * baseFee),
ter(tefBAD_QUORUM));
env.close();
BEAST_EXPECT(env.seq(alice) == aliceSeq);
// Test 1b: becky (2) + daria (1) meets quorum of 3
aliceSeq = env.seq(alice);
env(noop(alice),
msig(
{msigner(becky, msigner(bogie), msigner(demon)),
msigner(daria)}),
L(),
fee(5 * baseFee));
env.close();
BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
// Test 1c: cheri's nested signers must meet her quorum
aliceSeq = env.seq(alice);
env(noop(alice),
msig(
{msigner(
becky,
msigner(bogie),
msigner(demon)), // becky has a satisfied quorum
msigner(cheri, msigner(haunt))}), // but cheri does not
// (needs jinni too)
L(),
fee(5 * baseFee),
ter(tefBAD_QUORUM));
env.close();
BEAST_EXPECT(env.seq(alice) == aliceSeq);
// Test 1d: cheri with both signers meets her quorum
aliceSeq = env.seq(alice);
env(noop(alice),
msig(
{msigner(cheri, msigner(haunt), msigner(jinni)),
msigner(daria)}),
L(),
fee(5 * baseFee));
env.close();
BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
}
// Test Case 2: 3-level maximum depth with quorum at each level
{
// Level 2: phase needs direct signatures (no deeper nesting)
env(signers(phase, 2, {{acc10, 1}, {acc11, 1}, {acc12, 1}}));
// Level 1: jinni needs weighted signatures
env(signers(jinni, 3, {{phase, 2}, {shade, 2}, {spook, 1}}));
// Level 0: edgar needs 2 from weighted signers
env(signers(edgar, 2, {{jinni, 1}, {bogie, 1}, {demon, 1}}));
// Alice now requires edgar with weight 3
env(signers(alice, 3, {{edgar, 3}, {fiona, 2}}));
env.close();
// Test 2a: 3-level signing with phase signing directly (not through
// nested signers)
std::uint32_t aliceSeq = env.seq(alice);
env(noop(alice),
msig({
msigner(
edgar,
msigner(
jinni,
msigner(phase), // phase signs directly at level 3
msigner(shade)) // jinni quorum: 2+2 = 4 >= 3 ✓
) // edgar quorum: 1+0 = 1 < 2 ✗
}),
L(),
fee(4 * baseFee),
ter(tefBAD_QUORUM));
env.close();
BEAST_EXPECT(env.seq(alice) == aliceSeq);
// Test 2b: Edgar needs to meet his quorum too
aliceSeq = env.seq(alice);
env(noop(alice),
msig({
msigner(
edgar,
msigner(
jinni,
msigner(phase), // phase signs directly
msigner(shade)),
msigner(bogie)) // edgar quorum: 1+1 = 2 ✓
}),
L(),
fee(5 * baseFee));
env.close();
BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
// Test 2c: Use phase's signers (making it effectively 3-level from
// alice)
aliceSeq = env.seq(alice);
env(noop(alice),
msig({msigner(
edgar,
msigner(
jinni,
msigner(phase, msigner(acc10), msigner(acc11)),
msigner(spook)),
msigner(bogie))}),
L(),
fee(6 * baseFee));
env.close();
BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
}
// Test Case 3: Mixed levels - some direct, some nested at different
// depths (max 3)
{
// Set up mixed-level signing for alice
// grace has direct signers
env(signers(grace, 2, {{bogie, 1}, {demon, 1}}));
// henry has 2-level signers (henry -> becky -> bogie/demon)
env(signers(henry, 1, {{becky, 1}, {cheri, 1}}));
// edgar can be signed for by bogie
env(signers(edgar, 1, {{bogie, 1}, {shade, 1}}));
// Alice has mix of direct and nested signers at different weights
env(signers(
alice,
5,
{
{daria, 1}, // direct signer
{edgar, 2}, // has 2-level signers
{fiona, 1}, // direct signer
{grace, 2}, // has direct signers
{henry, 2} // has 2-level signers
}));
env.close();
// Test 3a: Mix of all levels meeting quorum exactly
std::uint32_t aliceSeq = env.seq(alice);
env(noop(alice),
msig({
msigner(daria), // weight 1, direct
msigner(edgar, msigner(bogie)), // weight 2, 2-level
msigner(grace, msigner(bogie), msigner(demon)) // weight 2,
// 2-level
}),
L(),
fee(6 * baseFee));
env.close();
BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
// Test 3b: 3-level signing through henry
aliceSeq = env.seq(alice);
env(noop(alice),
msig(
{msigner(fiona), // weight 1, direct
msigner(
grace, msigner(bogie)), // weight 2, 2-level (partial)
msigner(
henry, // weight 2, 3-level
msigner(becky, msigner(bogie), msigner(demon)))}),
L(),
fee(6 * baseFee),
ter(tefBAD_QUORUM)); // grace didn't meet quorum
env.close();
BEAST_EXPECT(env.seq(alice) == aliceSeq);
// Test 3c: Correct version with all quorums met
aliceSeq = env.seq(alice);
env(noop(alice),
msig(
{msigner(fiona), // weight 1
msigner(
edgar, msigner(bogie), msigner(shade)), // weight 2
msigner(
henry, // weight 2
msigner(becky, msigner(bogie), msigner(demon)))}),
L(),
fee(8 * baseFee)); // Total weight: 1+2+2 = 5 ✓
env.close();
BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
}
// Test Case 4: Complex scenario with maximum signers at mixed depths
// (max 3)
{
// Create a signing tree that uses close to maximum signers
// and tests weight accumulation across all levels
// Set up for alice: needs 15 out of possible 20 weight
env(signers(
alice,
15,
{
{becky, 3}, // will use 2-level
{cheri, 3}, // will use 2-level
{daria, 3}, // will use direct
{edgar, 3}, // will use 2-level
{fiona, 3}, // will use direct
{grace, 3}, // will use direct
{henry, 2} // will use 2-level
}));
env.close();
// Complex multi-level transaction just meeting quorum
std::uint32_t aliceSeq = env.seq(alice);
env(noop(alice),
msig({
msigner(
becky, // weight 3, 2-level
msigner(demon),
msigner(ghost)),
msigner(
cheri, // weight 3, 2-level
msigner(haunt),
msigner(jinni)),
msigner(daria), // weight 3, direct
msigner(
edgar, // weight 3, 2-level
msigner(bogie)),
msigner(grace) // weight 3, direct
}),
L(),
fee(10 * baseFee)); // Total weight: 3+3+3+3+3 = 15 ✓
env.close();
BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
// Test 4b: Test with henry using 3-level depth (maximum)
// First set up henry's chain properly
env(signers(henry, 1, {{jinni, 1}}));
env(signers(jinni, 2, {{acc10, 1}, {acc11, 1}}));
env.close();
aliceSeq = env.seq(alice);
env(noop(alice),
msig(
{msigner(
becky, // weight 3
msigner(demon)), // becky quorum not met!
msigner(
cheri, // weight 3
msigner(haunt),
msigner(jinni)),
msigner(daria), // weight 3
msigner(
henry, // weight 2, 3-level depth
msigner(jinni, msigner(acc10), msigner(acc11))),
msigner(
edgar, // weight 3
msigner(bogie),
msigner(shade))}),
L(),
fee(10 * baseFee),
ter(tefBAD_QUORUM)); // becky's quorum not met
env.close();
BEAST_EXPECT(env.seq(alice) == aliceSeq);
}
// Test Case 5: Edge case - single signer with maximum nesting (depth 3)
{
// Alice needs just one signer, but that signer uses depth up to 3
env(signers(alice, 1, {{becky, 1}}));
env.close();
std::uint32_t aliceSeq = env.seq(alice);
env(noop(alice),
msig({msigner(becky, msigner(demon), msigner(ghost))}),
L(),
fee(4 * baseFee));
env.close();
BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
// Now with 3-level depth (maximum allowed)
// Structure: alice -> becky -> cheri -> jinni (jinni signs
// directly)
env(signers(becky, 1, {{cheri, 1}}));
env(signers(cheri, 1, {{jinni, 1}}));
// Note: We do NOT add signers to jinni to keep max depth at 3
env.close();
aliceSeq = env.seq(alice);
env(noop(alice),
msig({msigner(
becky,
msigner(
cheri,
msigner(jinni)))}), // jinni signs directly (depth 3)
L(),
fee(4 * baseFee));
env.close();
BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
}
// Test Case 6: Simple cycle detection (A -> B -> A)
{
testcase("Cycle Detection - Simple");
// Reset signer lists for clean state
env(signers(alice, jtx::none));
env(signers(becky, jtx::none));
env.close();
// becky's signer list includes alice
// alice's signer list includes becky
// This creates: alice -> becky -> alice (cycle)
env(signers(alice, 1, {{becky, 1}, {bogie, 1}}));
env(signers(becky, 1, {{alice, 1}, {demon, 1}}));
env.close();
// Without cycle relaxation this would fail because:
// - alice needs becky (weight 1)
// - becky needs alice, but alice is ancestor -> cycle
// - becky's effective quorum relaxes since alice is unavailable
// - demon can satisfy becky's relaxed quorum
std::uint32_t aliceSeq = env.seq(alice);
env(noop(alice),
msig({msigner(becky, msigner(demon))}),
L(),
fee(4 * baseFee));
env.close();
BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
// Test that direct signer still works normally
aliceSeq = env.seq(alice);
env(noop(alice), msig({msigner(bogie)}), L(), fee(3 * baseFee));
env.close();
BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
}
// Test Case 7: The specific lockout scenario
// onyx:{jade, nova:{ruby:{jade, nova}, jade}}
// All have quorum 2, only jade can actually sign
{
testcase("Cycle Detection - Complex Lockout");
Account const onyx{"onyx", KeyType::secp256k1};
Account const nova{"nova", KeyType::ed25519};
Account const ruby{"ruby", KeyType::secp256k1};
Account const jade{"jade", KeyType::ed25519}; // phantom signer
env.fund(XRP(1000), onyx, nova, ruby);
env.close();
// Set up signer lists FIRST (before disabling master keys)
// ruby: {jade, nova} with quorum 2
env(signers(ruby, 2, {{jade, 1}, {nova, 1}}));
// nova: {ruby, jade} with quorum 2
env(signers(nova, 2, {{jade, 1}, {ruby, 1}}));
// onyx: {jade, nova} with quorum 2
env(signers(onyx, 2, {{jade, 1}, {nova, 1}}));
env.close();
// NOW disable master keys (signer lists provide alternative)
env(fset(onyx, asfDisableMaster), sig(onyx));
env(fset(nova, asfDisableMaster), sig(nova));
env(fset(ruby, asfDisableMaster), sig(ruby));
env.close();
// The signing tree for onyx:
// onyx (quorum 2) -> jade (weight 1) + nova (weight 1)
// nova (quorum 2) -> jade (weight 1) + ruby (weight 1)
// ruby (quorum 2) -> jade (weight 1) + nova (weight 1, CYCLE!)
//
// Without cycle detection: ruby needs nova, but nova is ancestor ->
// stuck With cycle detection:
// - At ruby level: nova is cyclic, cyclicWeight=1, totalWeight=2
// - maxAchievable = 2-1 = 1 < quorum(2), so effectiveQuorum -> 1
// - jade alone can satisfy ruby's relaxed quorum
// - ruby satisfied -> nova gets ruby's weight
// - nova: jade(1) + ruby(1) = 2 >= quorum(2) ✓
// - onyx: jade(1) + nova(1) = 2 >= quorum(2) ✓
std::uint32_t onyxSeq = env.seq(onyx);
env(noop(onyx),
msig(
{msigner(jade),
msigner(
nova,
msigner(jade),
msigner(
ruby, msigner(jade)))}), // nova is cyclic,
// skipped at ruby level
L(),
fee(6 * baseFee));
env.close();
BEAST_EXPECT(env.seq(onyx) == onyxSeq + 1);
}
// Test Case 8: Cycle where all signers are cyclic (effectiveQuorum ==
// 0)
{
testcase("Cycle Detection - Total Lockout");
Account const alpha{"alpha", KeyType::secp256k1};
Account const beta{"beta", KeyType::ed25519};
Account const gamma{"gamma", KeyType::secp256k1};
env.fund(XRP(1000), alpha, beta, gamma);
env.close();
// Set up pure cycle signer lists FIRST
env(signers(alpha, 1, {{beta, 1}}));
env(signers(beta, 1, {{gamma, 1}}));
env(signers(gamma, 1, {{alpha, 1}}));
env.close();
// NOW disable master keys
env(fset(alpha, asfDisableMaster), sig(alpha));
env(fset(beta, asfDisableMaster), sig(beta));
env(fset(gamma, asfDisableMaster), sig(gamma));
env.close();
// This is a true lockout - no valid signing path exists.
// gamma appears as a leaf signer but has master disabled ->
// tefMASTER_DISABLED (The cycle detection would return
// tefBAD_QUORUM if gamma were nested, but there's no way to
// construct such a transaction since gamma's only signer is alpha,
// which is what we're trying to sign for)
std::uint32_t alphaSeq = env.seq(alpha);
env(noop(alpha),
msig({msigner(
beta,
msigner(gamma))}), // gamma can't sign - master disabled
L(),
fee(4 * baseFee),
ter(tefMASTER_DISABLED));
env.close();
BEAST_EXPECT(env.seq(alpha) == alphaSeq);
}
// Test Case 9: Cycle at depth 3 (near max depth)
{
testcase("Cycle Detection - Deep Cycle");
// Reset signer lists
env(signers(alice, jtx::none));
env(signers(becky, jtx::none));
env(signers(cheri, jtx::none));
env(signers(daria, jtx::none));
env.close();
// Structure: alice -> becky -> cheri -> daria -> alice (cycle at
// depth 4)
env(signers(alice, 1, {{becky, 1}, {bogie, 1}}));
env(signers(becky, 1, {{cheri, 1}}));
env(signers(cheri, 1, {{daria, 1}}));
env(signers(daria, 1, {{alice, 1}, {demon, 1}}));
env.close();
// At depth 4, daria needs alice but alice is ancestor
// daria's quorum relaxes, demon can satisfy
std::uint32_t aliceSeq = env.seq(alice);
env(noop(alice),
msig({msigner(
becky, msigner(cheri, msigner(daria, msigner(demon))))}),
L(),
fee(6 * baseFee));
env.close();
BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
}
// Test Case 10: Multiple independent cycles in same tree
{
testcase("Cycle Detection - Multiple Cycles");
// Reset signer lists
env(signers(alice, jtx::none));
env(signers(becky, jtx::none));
env(signers(cheri, jtx::none));
env.close();
// alice -> {becky, cheri}
// becky -> {alice, bogie} (cycle back to alice)
// cheri -> {alice, demon} (another cycle back to alice)
env(signers(alice, 2, {{becky, 1}, {cheri, 1}}));
env(signers(becky, 2, {{alice, 1}, {bogie, 1}}));
env(signers(cheri, 2, {{alice, 1}, {demon, 1}}));
env.close();
// Both becky and cheri have cycles back to alice
// Both need their quorums relaxed
// bogie satisfies becky, demon satisfies cheri
std::uint32_t aliceSeq = env.seq(alice);
env(noop(alice),
msig(
{msigner(becky, msigner(bogie)),
msigner(cheri, msigner(demon))}),
L(),
fee(6 * baseFee));
env.close();
BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
}
// Test Case 11: Cycle with sufficient non-cyclic weight (no relaxation
// needed)
{
testcase("Cycle Detection - No Relaxation Needed");
// Reset signer lists
env(signers(alice, jtx::none));
env(signers(becky, jtx::none));
env.close();
// becky has alice in signer list but also has enough other signers
env(signers(alice, 1, {{becky, 1}}));
env(signers(becky, 2, {{alice, 1}, {bogie, 1}, {demon, 1}}));
env.close();
// becky quorum is 2, alice is cyclic (weight 1)
// totalWeight = 3, cyclicWeight = 1, maxAchievable = 2 >= quorum
// No relaxation needed, bogie + demon satisfy quorum normally
std::uint32_t aliceSeq = env.seq(alice);
env(noop(alice),
msig({msigner(becky, msigner(bogie), msigner(demon))}),
L(),
fee(5 * baseFee));
env.close();
BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
// Should fail if only one non-cyclic signer provided
aliceSeq = env.seq(alice);
env(noop(alice),
msig({msigner(becky, msigner(bogie))}),
L(),
fee(4 * baseFee),
ter(tefBAD_QUORUM));
env.close();
BEAST_EXPECT(env.seq(alice) == aliceSeq);
}
// Test Case 12: Partial cycle - one branch cyclic, one not
{
testcase("Cycle Detection - Partial Cycle");
// Reset signer lists
env(signers(alice, jtx::none));
env(signers(becky, jtx::none));
env(signers(cheri, jtx::none));
env.close();
// alice -> {becky, cheri}
// becky -> {alice, bogie} (cyclic)
// cheri -> {daria} (not cyclic)
env(signers(alice, 2, {{becky, 1}, {cheri, 1}}));
env(signers(becky, 1, {{alice, 1}, {bogie, 1}}));
env(signers(cheri, 1, {{daria, 1}}));
env.close();
// becky's branch has cycle, cheri's doesn't
// Both contribute to alice's quorum
std::uint32_t aliceSeq = env.seq(alice);
env(noop(alice),
msig(
{msigner(becky, msigner(bogie)), // relaxed quorum
msigner(cheri, msigner(daria))}), // normal quorum
L(),
fee(6 * baseFee));
env.close();
BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
}
// Test Case 13: Diamond pattern with cycle
{
testcase("Cycle Detection - Diamond Pattern");
// Reset signer lists
env(signers(alice, jtx::none));
env(signers(becky, jtx::none));
env(signers(cheri, jtx::none));
env(signers(daria, jtx::none));
env.close();
// alice -> {becky, cheri}
// becky -> {daria}
// cheri -> {daria}
// daria -> {alice, bogie} (cycle through both paths)
env(signers(alice, 2, {{becky, 1}, {cheri, 1}}));
env(signers(becky, 1, {{daria, 1}}));
env(signers(cheri, 1, {{daria, 1}}));
env(signers(daria, 1, {{alice, 1}, {bogie, 1}}));
env.close();
// Both paths converge at daria, which cycles back to alice
std::uint32_t aliceSeq = env.seq(alice);
env(noop(alice),
msig(
{msigner(becky, msigner(daria, msigner(bogie))),
msigner(cheri, msigner(daria, msigner(bogie)))}),
L(),
fee(7 * baseFee));
env.close();
BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
}
// Test Case 14: Cycle requiring maximum quorum relaxation
{
testcase("Cycle Detection - Maximum Relaxation");
Account const omega{"omega", KeyType::secp256k1};
Account const sigma{"sigma", KeyType::ed25519};
env.fund(XRP(1000), omega, sigma);
env.close();
// Reset alice and becky signer lists
env(signers(alice, jtx::none));
env(signers(becky, jtx::none));
env.close();
// Set up signer lists FIRST
env(signers(sigma, 1, {{omega, 1}, {bogie, 1}}));
env(signers(omega, 3, {{sigma, 2}, {alice, 1}, {becky, 1}}));
env(signers(alice, 1, {{omega, 1}, {demon, 1}}));
env(signers(becky, 1, {{omega, 1}, {ghost, 1}}));
env.close();
// NOW disable master keys
env(fset(omega, asfDisableMaster), sig(omega));
env(fset(sigma, asfDisableMaster), sig(sigma));
env.close();
// From omega's perspective when signing for omega:
// - sigma: needs omega (cyclic), so relaxes to bogie only
// - alice: needs omega (cyclic), so relaxes to demon only
// - becky: needs omega (cyclic), so relaxes to ghost only
// All signers need relaxation but can be satisfied
std::uint32_t omegaSeq = env.seq(omega);
env(noop(omega),
msig(
{msigner(alice, msigner(demon)),
msigner(becky, msigner(ghost)),
msigner(sigma, msigner(bogie))}),
L(),
fee(7 * baseFee));
env.close();
BEAST_EXPECT(env.seq(omega) == omegaSeq + 1);
}
// Test Case 15: Cycle at exact max depth boundary
{
testcase("Cycle Detection - Max Depth Boundary");
// Reset signer lists
env(signers(alice, jtx::none));
env(signers(becky, jtx::none));
env(signers(cheri, jtx::none));
env(signers(daria, jtx::none));
env(signers(edgar, jtx::none));
env.close();
// Depth 4 is max: alice(1) -> becky(2) -> cheri(3) -> daria(4)
// daria cycles back but we're at max depth
env(signers(alice, 1, {{becky, 1}}));
env(signers(becky, 1, {{cheri, 1}}));
env(signers(cheri, 1, {{daria, 1}}));
env(signers(daria, 1, {{alice, 1}, {bogie, 1}}));
env.close();
// This should work - cycle detected and relaxed at depth 4
std::uint32_t aliceSeq = env.seq(alice);
env(noop(alice),
msig({msigner(
becky, msigner(cheri, msigner(daria, msigner(bogie))))}),
L(),
fee(6 * baseFee));
env.close();
BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
// Now try to exceed depth (add edgar at depth 5)
env(signers(daria, 1, {{edgar, 1}}));
env(signers(edgar, 1, {{bogie, 1}}));
env.close();
// Transaction structure is rejected at preflight for exceeding
// nesting limits
aliceSeq = env.seq(alice);
env(noop(alice),
msig({msigner(
becky,
msigner(
cheri,
msigner(daria, msigner(edgar, msigner(bogie)))))}),
L(),
fee(7 * baseFee),
ter(temMALFORMED)); // Rejected at preflight for excessive
// nesting
env.close();
BEAST_EXPECT(env.seq(alice) == aliceSeq);
}
}
void
test_signerListSetFlags(FeatureBitset features)
{
@@ -2503,7 +1709,6 @@ public:
test_signForHash(features);
test_signersWithTickets(features);
test_signersWithTags(features);
test_nestedMultiSign(features);
}
void
@@ -2516,11 +1721,8 @@ public:
// featureMultiSignReserve. Limits on the number of signers
// changes based on featureExpandedSignerList. Test both with and
// without.
testAll(
all - featureMultiSignReserve - featureExpandedSignerList -
featureNestedMultiSign);
testAll(all - featureExpandedSignerList - featureNestedMultiSign);
testAll(all - featureNestedMultiSign);
testAll(all - featureMultiSignReserve - featureExpandedSignerList);
testAll(all - featureExpandedSignerList);
testAll(all);
test_signerListSetFlags(all);

View File

@@ -66,45 +66,15 @@ signers(Account const& account, none_t)
//------------------------------------------------------------------------------
// Helper function to recursively sort nested signers
void
sortSignersRecursive(std::vector<msig::SignerPtr>& signers)
msig::msig(std::vector<msig::Reg> signers_) : signers(std::move(signers_))
{
// Sort current level by account ID
// Signatures must be applied in sorted order.
std::sort(
signers.begin(),
signers.end(),
[](msig::SignerPtr const& lhs, msig::SignerPtr const& rhs) {
return lhs->id() < rhs->id();
[](msig::Reg const& lhs, msig::Reg const& rhs) {
return lhs.acct.id() < rhs.acct.id();
});
// Recursively sort nested signers for each signer at this level
for (auto& signer : signers)
{
if (signer->isNested() && !signer->nested.empty())
{
sortSignersRecursive(signer->nested);
}
}
}
msig::msig(std::vector<msig::SignerPtr> signers_) : signers(std::move(signers_))
{
// Recursively sort all signers at all nesting levels
// This ensures account IDs are in strictly ascending order at each level
sortSignersRecursive(signers);
}
msig::msig(std::vector<msig::Reg> signers_)
{
// Convert Reg vector to SignerPtr vector for backward compatibility
signers.reserve(signers_.size());
for (auto const& s : signers_)
signers.push_back(s.toSigner());
// Recursively sort all signers at all nesting levels
// This ensures account IDs are in strictly ascending order at each level
sortSignersRecursive(signers);
}
void
@@ -123,47 +93,19 @@ msig::operator()(Env& env, JTx& jt) const
env.test.log << pretty(jtx.jv) << std::endl;
Rethrow();
}
// Recursive function to build signer JSON
std::function<Json::Value(SignerPtr const&)> buildSignerJson;
buildSignerJson = [&](SignerPtr const& signer) -> Json::Value {
Json::Value jo;
jo[jss::Account] = signer->acct.human();
if (signer->isNested())
{
// For nested signers, we use the already-sorted nested vector
// (sorted during construction via sortSignersRecursive)
// This ensures account IDs are in strictly ascending order
auto& subJs = jo[sfSigners.getJsonName()];
for (std::size_t i = 0; i < signer->nested.size(); ++i)
{
auto& subJo = subJs[i][sfSigner.getJsonName()];
subJo = buildSignerJson(signer->nested[i]);
}
}
else
{
// This is a leaf signer - add signature
jo[jss::SigningPubKey] = strHex(signer->sig.pk().slice());
Serializer ss{buildMultiSigningData(*st, signer->acct.id())};
auto const sig = ripple::sign(
*publicKeyType(signer->sig.pk().slice()),
signer->sig.sk(),
ss.slice());
jo[sfTxnSignature.getJsonName()] =
strHex(Slice{sig.data(), sig.size()});
}
return jo;
};
auto& js = jtx[sfSigners.getJsonName()];
for (std::size_t i = 0; i < mySigners.size(); ++i)
{
auto const& e = mySigners[i];
auto& jo = js[i][sfSigner.getJsonName()];
jo = buildSignerJson(mySigners[i]);
jo[jss::Account] = e.acct.human();
jo[jss::SigningPubKey] = strHex(e.sig.pk().slice());
Serializer ss{buildMultiSigningData(*st, e.acct.id())};
auto const sig = ripple::sign(
*publicKeyType(e.sig.pk().slice()), e.sig.sk(), ss.slice());
jo[sfTxnSignature.getJsonName()] =
strHex(Slice{sig.data(), sig.size()});
}
};
}

View File

@@ -21,7 +21,6 @@
#define RIPPLE_TEST_JTX_MULTISIGN_H_INCLUDED
#include <cstdint>
#include <memory>
#include <optional>
#include <test/jtx/Account.h>
#include <test/jtx/amount.h>
@@ -66,48 +65,6 @@ signers(Account const& account, none_t);
class msig
{
public:
// Recursive signer structure
struct Signer
{
Account acct;
Account sig; // For leaf signers (same as acct for master key)
std::vector<std::shared_ptr<Signer>> nested; // For nested signers
// Leaf signer constructor (regular signing)
Signer(Account const& masterSig) : acct(masterSig), sig(masterSig)
{
}
// Leaf signer constructor (with different signing key)
Signer(Account const& acct_, Account const& regularSig)
: acct(acct_), sig(regularSig)
{
}
// Nested signer constructor
Signer(
Account const& acct_,
std::vector<std::shared_ptr<Signer>> nested_)
: acct(acct_), nested(std::move(nested_))
{
}
bool
isNested() const
{
return !nested.empty();
}
AccountID
id() const
{
return acct.id();
}
};
using SignerPtr = std::shared_ptr<Signer>;
// For backward compatibility
struct Reg
{
Account acct;
@@ -116,13 +73,16 @@ public:
Reg(Account const& masterSig) : acct(masterSig), sig(masterSig)
{
}
Reg(Account const& acct_, Account const& regularSig)
: acct(acct_), sig(regularSig)
{
}
Reg(char const* masterSig) : acct(masterSig), sig(masterSig)
{
}
Reg(char const* acct_, char const* regularSig)
: acct(acct_), sig(regularSig)
{
@@ -133,25 +93,13 @@ public:
{
return acct < rhs.acct;
}
// Convert to Signer
SignerPtr
toSigner() const
{
return std::make_shared<Signer>(acct, sig);
}
};
std::vector<SignerPtr> signers;
std::vector<Reg> signers;
public:
// Direct constructor with SignerPtr vector
msig(std::vector<SignerPtr> signers_);
// Backward compatibility constructor
msig(std::vector<Reg> signers_);
// Variadic constructor for backward compatibility
template <class AccountType, class... Accounts>
explicit msig(AccountType&& a0, Accounts&&... aN)
: msig{std::vector<Reg>{
@@ -164,30 +112,6 @@ public:
operator()(Env&, JTx& jt) const;
};
// Helper functions to create signers - renamed to avoid conflict with sig()
// transaction modifier
inline msig::SignerPtr
msigner(Account const& acct)
{
return std::make_shared<msig::Signer>(acct);
}
inline msig::SignerPtr
msigner(Account const& acct, Account const& signingKey)
{
return std::make_shared<msig::Signer>(acct, signingKey);
}
// Create nested signer with initializer list
template <typename... Args>
inline msig::SignerPtr
msigner(Account const& acct, Args&&... args)
{
std::vector<msig::SignerPtr> nested;
(nested.push_back(std::forward<Args>(args)), ...);
return std::make_shared<msig::Signer>(acct, std::move(nested));
}
//------------------------------------------------------------------------------
/** The number of signer lists matches. */

View File

@@ -332,7 +332,6 @@ multi_runner_child::run_multi(Pred pred)
{
if (!pred(*t))
continue;
try
{
failed = run(*t) || failed;