diff --git a/include/xrpl/protocol/STObject.h b/include/xrpl/protocol/STObject.h index 1c22b08aba..84706c5833 100644 --- a/include/xrpl/protocol/STObject.h +++ b/include/xrpl/protocol/STObject.h @@ -244,6 +244,9 @@ public: getFieldPathSet(SField const& field) const; STVector256 const& getFieldV256(SField const& field) const; + // If not found, returns an object constructed with the given field + STObject + getFieldObject(SField const& field) const; STArray const& getFieldArray(SField const& field) const; STCurrency const& @@ -390,6 +393,8 @@ public: setFieldV256(SField const& field, STVector256 const& v); void setFieldArray(SField const& field, STArray const& v); + void + setFieldObject(SField const& field, STObject const& v); template void diff --git a/include/xrpl/protocol/STTx.h b/include/xrpl/protocol/STTx.h index f0d2157283..ac0223ee77 100644 --- a/include/xrpl/protocol/STTx.h +++ b/include/xrpl/protocol/STTx.h @@ -87,8 +87,14 @@ public: getFullText() const override; // Outer transaction functions / signature functions. + static Blob + getSignature(STObject const& sigObject); + Blob - getSignature() const; + getSignature() const + { + return getSignature(*this); + } uint256 getSigningHash() const; @@ -119,13 +125,20 @@ public: getJson(JsonOptions options, bool binary) const; void - sign(PublicKey const& publicKey, SecretKey const& secretKey); + sign( + PublicKey const& publicKey, + SecretKey const& secretKey, + std::optional> signatureTarget = + {}); - /** Check the signature. - @return `true` if valid signature. If invalid, the error message string. - */ enum class RequireFullyCanonicalSig : bool { no, yes }; + /** Check the signature. + @param requireCanonicalSig If `true`, check that the signature is fully + canonical. If `false`, only check that the signature is valid. + @param rules The current ledger rules. + @return `true` if valid signature. If invalid, the error message string. + */ Expected checkSign(RequireFullyCanonicalSig requireCanonicalSig, Rules const& rules) const; @@ -150,17 +163,34 @@ public: char status, std::string const& escapedMetaData) const; - std::vector + std::vector const& getBatchTransactionIDs() const; private: + /** Check the signature. + @param requireCanonicalSig If `true`, check that the signature is fully + canonical. If `false`, only check that the signature is valid. + @param rules The current ledger rules. + @param sigObject Reference to object that contains the signature fields. + Will be *this more often than not. + @return `true` if valid signature. If invalid, the error message string. + */ Expected - checkSingleSign(RequireFullyCanonicalSig requireCanonicalSig) const; + checkSign( + RequireFullyCanonicalSig requireCanonicalSig, + Rules const& rules, + STObject const& sigObject) const; + + Expected + checkSingleSign( + RequireFullyCanonicalSig requireCanonicalSig, + STObject const& sigObject) const; Expected checkMultiSign( RequireFullyCanonicalSig requireCanonicalSig, - Rules const& rules) const; + Rules const& rules, + STObject const& sigObject) const; Expected checkBatchSingleSign( @@ -179,7 +209,7 @@ private: move(std::size_t n, void* buf) override; friend class detail::STVar; - mutable std::vector batch_txn_ids_; + mutable std::vector batchTxnIds_; }; bool diff --git a/include/xrpl/protocol/jss.h b/include/xrpl/protocol/jss.h index 8609aedaef..bb5af1fc4d 100644 --- a/include/xrpl/protocol/jss.h +++ b/include/xrpl/protocol/jss.h @@ -569,6 +569,7 @@ JSS(settle_delay); // out: AccountChannels JSS(severity); // in: LogLevel JSS(shares); // out: VaultInfo JSS(signature); // out: NetworkOPs, ChannelAuthorize +JSS(signature_target); // in: TransactionSign JSS(signature_verified); // out: ChannelVerify JSS(signing_key); // out: NetworkOPs JSS(signing_keys); // out: ValidatorList diff --git a/src/libxrpl/protocol/STObject.cpp b/src/libxrpl/protocol/STObject.cpp index 77e5fd1ad9..8fb21a638d 100644 --- a/src/libxrpl/protocol/STObject.cpp +++ b/src/libxrpl/protocol/STObject.cpp @@ -688,6 +688,16 @@ STObject::getFieldV256(SField const& field) const return getFieldByConstRef(field, empty); } +STObject +STObject::getFieldObject(SField const& field) const +{ + STObject const empty{field}; + auto ret = getFieldByConstRef(field, empty); + if (ret != empty) + ret.applyTemplateFromSField(field); + return ret; +} + STArray const& STObject::getFieldArray(SField const& field) const { @@ -833,6 +843,12 @@ STObject::setFieldArray(SField const& field, STArray const& v) setFieldUsingAssignment(field, v); } +void +STObject::setFieldObject(SField const& field, STObject const& v) +{ + setFieldUsingAssignment(field, v); +} + Json::Value STObject::getJson(JsonOptions options) const { diff --git a/src/libxrpl/protocol/STTx.cpp b/src/libxrpl/protocol/STTx.cpp index 8be8f906a5..7326f48424 100644 --- a/src/libxrpl/protocol/STTx.cpp +++ b/src/libxrpl/protocol/STTx.cpp @@ -200,11 +200,11 @@ STTx::getSigningHash() const } Blob -STTx::getSignature() const +STTx::getSignature(STObject const& sigObject) { try { - return getFieldVL(sfTxnSignature); + return sigObject.getFieldVL(sfTxnSignature); } catch (std::exception const&) { @@ -234,35 +234,68 @@ STTx::getSeqValue() const } void -STTx::sign(PublicKey const& publicKey, SecretKey const& secretKey) +STTx::sign( + PublicKey const& publicKey, + SecretKey const& secretKey, + std::optional> signatureTarget) { auto const data = getSigningData(*this); auto const sig = ripple::sign(publicKey, secretKey, makeSlice(data)); - setFieldVL(sfTxnSignature, sig); + if (signatureTarget) + { + auto& target = peekFieldObject(*signatureTarget); + target.setFieldVL(sfTxnSignature, sig); + } + else + { + setFieldVL(sfTxnSignature, sig); + } tid_ = getHash(HashPrefix::transactionID); } +Expected +STTx::checkSign( + RequireFullyCanonicalSig requireCanonicalSig, + Rules const& rules, + STObject const& sigObject) const +{ + try + { + // Determine whether we're single- or multi-signing by looking + // at the SigningPubKey. If it's empty we must be + // multi-signing. Otherwise we're single-signing. + + Blob const& signingPubKey = sigObject.getFieldVL(sfSigningPubKey); + return signingPubKey.empty() + ? checkMultiSign(requireCanonicalSig, rules, sigObject) + : checkSingleSign(requireCanonicalSig, sigObject); + } + catch (std::exception const&) + { + } + return Unexpected("Internal signature check failure."); +} + Expected STTx::checkSign( RequireFullyCanonicalSig requireCanonicalSig, Rules const& rules) const { - try + if (auto const ret = checkSign(requireCanonicalSig, rules, *this); !ret) + return ret; + + /* Placeholder for field that will be added by Lending Protocol + if (isFieldPresent(sfCounterpartySignature)) { - // Determine whether we're single- or multi-signing by looking - // 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, rules) - : checkSingleSign(requireCanonicalSig); + auto const counterSig = getFieldObject(sfCounterpartySignature); + if (auto const ret = checkSign(requireCanonicalSig, rules, counterSig); + !ret) + return Unexpected("Counterparty: " + ret.error()); } - catch (std::exception const&) - { - } - return Unexpected("Internal signature check failure."); + */ + return {}; } Expected @@ -382,23 +415,23 @@ STTx::getMetaSQL( static Expected singleSignHelper( - STObject const& signer, + STObject const& sigObject, Slice const& data, bool const fullyCanonical) { // We don't allow both a non-empty sfSigningPubKey and an sfSigners. // That would allow the transaction to be signed two ways. So if both // fields are present the signature is invalid. - if (signer.isFieldPresent(sfSigners)) + if (sigObject.isFieldPresent(sfSigners)) return Unexpected("Cannot both single- and multi-sign."); bool validSig = false; try { - auto const spk = signer.getFieldVL(sfSigningPubKey); + auto const spk = sigObject.getFieldVL(sfSigningPubKey); if (publicKeyType(makeSlice(spk))) { - Blob const signature = signer.getFieldVL(sfTxnSignature); + Blob const signature = sigObject.getFieldVL(sfTxnSignature); validSig = verify( PublicKey(makeSlice(spk)), data, @@ -418,12 +451,14 @@ singleSignHelper( } Expected -STTx::checkSingleSign(RequireFullyCanonicalSig requireCanonicalSig) const +STTx::checkSingleSign( + RequireFullyCanonicalSig requireCanonicalSig, + STObject const& sigObject) const { auto const data = getSigningData(*this); bool const fullyCanonical = (getFlags() & tfFullyCanonicalSig) || (requireCanonicalSig == STTx::RequireFullyCanonicalSig::yes); - return singleSignHelper(*this, makeSlice(data), fullyCanonical); + return singleSignHelper(sigObject, makeSlice(data), fullyCanonical); } Expected @@ -440,31 +475,29 @@ STTx::checkBatchSingleSign( Expected multiSignHelper( - STObject const& signerObj, + STObject const& sigObject, + std::optional txnAccountID, bool const fullyCanonical, std::function makeMsg, Rules const& rules) { // Make sure the MultiSigners are present. Otherwise they are not // attempting multi-signing and we just have a bad SigningPubKey. - if (!signerObj.isFieldPresent(sfSigners)) + if (!sigObject.isFieldPresent(sfSigners)) return Unexpected("Empty SigningPubKey."); // We don't allow both an sfSigners and an sfTxnSignature. Both fields // being present would indicate that the transaction is signed both ways. - if (signerObj.isFieldPresent(sfTxnSignature)) + if (sigObject.isFieldPresent(sfTxnSignature)) return Unexpected("Cannot both single- and multi-sign."); - STArray const& signers{signerObj.getFieldArray(sfSigners)}; + STArray const& signers{sigObject.getFieldArray(sfSigners)}; // There are well known bounds that the number of signers must be within. if (signers.size() < STTx::minMultiSigners || signers.size() > STTx::maxMultiSigners(&rules)) return Unexpected("Invalid Signers array size."); - // We also use the sfAccount field inside the loop. Get it once. - auto const txnAccountID = signerObj.getAccountID(sfAccount); - // Signers must be in sorted order by AccountID. AccountID lastAccountID(beast::zero); @@ -472,8 +505,10 @@ multiSignHelper( { auto const accountID = signer.getAccountID(sfAccount); - // The account owner may not multisign for themselves. - if (accountID == txnAccountID) + // The account owner may not usually multisign for themselves. + // If they can, txnAccountID will be unseated, which is not equal to any + // value. + if (txnAccountID == accountID) return Unexpected("Invalid multisigner."); // No duplicate signers allowed. @@ -489,6 +524,7 @@ multiSignHelper( // Verify the signature. bool validSig = false; + std::optional errorWhat; try { auto spk = signer.getFieldVL(sfSigningPubKey); @@ -502,15 +538,16 @@ multiSignHelper( fullyCanonical); } } - catch (std::exception const&) + catch (std::exception const& e) { // We assume any problem lies with the signature. validSig = false; + errorWhat = e.what(); } if (!validSig) return Unexpected( std::string("Invalid signature on account ") + - toBase58(accountID) + "."); + toBase58(accountID) + errorWhat.value_or("") + "."); } // All signatures verified. return {}; @@ -532,8 +569,9 @@ STTx::checkBatchMultiSign( serializeBatch(dataStart, getFlags(), getBatchTransactionIDs()); return multiSignHelper( batchSigner, + std::nullopt, fullyCanonical, - [&dataStart](AccountID const& accountID) mutable -> Serializer { + [&dataStart](AccountID const& accountID) -> Serializer { Serializer s = dataStart; finishMultiSigningData(accountID, s); return s; @@ -544,19 +582,27 @@ STTx::checkBatchMultiSign( Expected STTx::checkMultiSign( RequireFullyCanonicalSig requireCanonicalSig, - Rules const& rules) const + Rules const& rules, + STObject const& sigObject) const { bool const fullyCanonical = (getFlags() & tfFullyCanonicalSig) || (requireCanonicalSig == RequireFullyCanonicalSig::yes); + // Used inside the loop in multiSignHelper to enforce that + // the account owner may not multisign for themselves. + auto const txnAccountID = &sigObject != this + ? std::nullopt + : std::optional(getAccountID(sfAccount)); + // We can ease the computational load inside the loop a bit by // pre-constructing part of the data that we hash. Fill a Serializer // with the stuff that stays constant from signature to signature. Serializer dataStart = startMultiSigningData(*this); return multiSignHelper( - *this, + sigObject, + txnAccountID, fullyCanonical, - [&dataStart](AccountID const& accountID) mutable -> Serializer { + [&dataStart](AccountID const& accountID) -> Serializer { Serializer s = dataStart; finishMultiSigningData(accountID, s); return s; @@ -569,7 +615,7 @@ STTx::checkMultiSign( * * This function returns a vector of transaction IDs by extracting them from * the field array `sfRawTransactions` within the STTx. If the batch - * transaction IDs have already been computed and cached in `batch_txn_ids_`, + * transaction IDs have already been computed and cached in `batchTxnIds_`, * it returns the cached vector. Otherwise, it computes the transaction IDs, * caches them, and then returns the vector. * @@ -579,7 +625,7 @@ STTx::checkMultiSign( * empty and that the size of the computed batch transaction IDs matches the * size of the `sfRawTransactions` field array. */ -std::vector +std::vector const& STTx::getBatchTransactionIDs() const { XRPL_ASSERT( @@ -588,16 +634,20 @@ STTx::getBatchTransactionIDs() const XRPL_ASSERT( getFieldArray(sfRawTransactions).size() != 0, "STTx::getBatchTransactionIDs : empty raw transactions"); - if (batch_txn_ids_.size() != 0) - return batch_txn_ids_; - for (STObject const& rb : getFieldArray(sfRawTransactions)) - batch_txn_ids_.push_back(rb.getHash(HashPrefix::transactionID)); + // The list of inner ids is built once, then reused on subsequent calls. + // After the list is built, it must always have the same size as the array + // `sfRawTransactions`. The assert below verifies that. + if (batchTxnIds_.size() == 0) + { + for (STObject const& rb : getFieldArray(sfRawTransactions)) + batchTxnIds_.push_back(rb.getHash(HashPrefix::transactionID)); + } XRPL_ASSERT( - batch_txn_ids_.size() == getFieldArray(sfRawTransactions).size(), + batchTxnIds_.size() == getFieldArray(sfRawTransactions).size(), "STTx::getBatchTransactionIDs : batch transaction IDs size mismatch"); - return batch_txn_ids_; + return batchTxnIds_; } //------------------------------------------------------------------------------ diff --git a/src/test/jtx/JTx.h b/src/test/jtx/JTx.h index 198839dd28..36127f1843 100644 --- a/src/test/jtx/JTx.h +++ b/src/test/jtx/JTx.h @@ -54,7 +54,11 @@ struct JTx bool fill_sig = true; bool fill_netid = true; std::shared_ptr stx; - std::function signer; + // Functions that sign the transaction from the Account + std::vector> mainSigners; + // Functions that sign something else after the mainSigners, such as + // sfCounterpartySignature + std::vector> postSigners; JTx() = default; JTx(JTx const&) = default; diff --git a/src/test/jtx/TestHelpers.h b/src/test/jtx/TestHelpers.h index a7bdbb9b9f..5d919add47 100644 --- a/src/test/jtx/TestHelpers.h +++ b/src/test/jtx/TestHelpers.h @@ -618,7 +618,7 @@ create( } // namespace check -static constexpr FeeLevel64 baseFeeLevel{256}; +static constexpr FeeLevel64 baseFeeLevel{TxQ::baseLevel}; static constexpr FeeLevel64 minEscalationFeeLevel = baseFeeLevel * 500; template diff --git a/src/test/jtx/amount.h b/src/test/jtx/amount.h index a793f3a287..f14a65f6b8 100644 --- a/src/test/jtx/amount.h +++ b/src/test/jtx/amount.h @@ -213,14 +213,16 @@ public: template PrettyAmount - operator()(T v) const + operator()(T v, Number::rounding_mode rounding = Number::getround()) const { - return operator()(Number(v)); + return operator()(Number(v), rounding); } PrettyAmount - operator()(Number v) const + operator()(Number v, Number::rounding_mode rounding = Number::getround()) + const { + NumberRoundModeGuard mg(rounding); STAmount amount{asset_, v * scale_}; return {amount, ""}; } diff --git a/src/test/jtx/impl/Env.cpp b/src/test/jtx/impl/Env.cpp index f17f6cb39d..ce5f6f150c 100644 --- a/src/test/jtx/impl/Env.cpp +++ b/src/test/jtx/impl/Env.cpp @@ -34,6 +34,7 @@ #include #include +#include #include #include #include @@ -531,8 +532,22 @@ void Env::autofill_sig(JTx& jt) { auto& jv = jt.jv; - if (jt.signer) - return jt.signer(*this, jt); + + scope_success success([&]() { + // Call all the post-signers after the main signers or autofill are done + for (auto const& signer : jt.postSigners) + signer(*this, jt); + }); + + // Call all the main signers + if (!jt.mainSigners.empty()) + { + for (auto const& signer : jt.mainSigners) + signer(*this, jt); + return; + } + + // If the sig is still needed, get it here. if (!jt.fill_sig) return; auto const account = jv.isMember(sfDelegate.jsonName) diff --git a/src/test/jtx/impl/mpt.cpp b/src/test/jtx/impl/mpt.cpp index f2f51492e3..aaa5e433f2 100644 --- a/src/test/jtx/impl/mpt.cpp +++ b/src/test/jtx/impl/mpt.cpp @@ -507,7 +507,7 @@ MPTTester::getFlags(std::optional const& holder) const } MPT -MPTTester::operator[](std::string const& name) +MPTTester::operator[](std::string const& name) const { return MPT(name, issuanceID()); } diff --git a/src/test/jtx/impl/multisign.cpp b/src/test/jtx/impl/multisign.cpp index 6ed6df6804..fc6163a2f3 100644 --- a/src/test/jtx/impl/multisign.cpp +++ b/src/test/jtx/impl/multisign.cpp @@ -69,8 +69,15 @@ void msig::operator()(Env& env, JTx& jt) const { auto const mySigners = signers; - jt.signer = [mySigners, &env](Env&, JTx& jtx) { - jtx[sfSigningPubKey.getJsonName()] = ""; + auto callback = [subField = subField, mySigners, &env](Env&, JTx& jtx) { + // Where to put the signature. Supports sfCounterPartySignature. + auto& sigObject = subField ? jtx[*subField] : jtx.jv; + + // The signing pub key is only required at the top level. + if (!subField) + sigObject[sfSigningPubKey] = ""; + else if (sigObject.isNull()) + sigObject = Json::Value(Json::objectValue); std::optional st; try { @@ -81,7 +88,7 @@ msig::operator()(Env& env, JTx& jt) const env.test.log << pretty(jtx.jv) << std::endl; Rethrow(); } - auto& js = jtx[sfSigners.getJsonName()]; + auto& js = sigObject[sfSigners]; for (std::size_t i = 0; i < mySigners.size(); ++i) { auto const& e = mySigners[i]; @@ -96,6 +103,10 @@ msig::operator()(Env& env, JTx& jt) const strHex(Slice{sig.data(), sig.size()}); } }; + if (!subField) + jt.mainSigners.emplace_back(callback); + else + jt.postSigners.emplace_back(callback); } } // namespace jtx diff --git a/src/test/jtx/impl/sig.cpp b/src/test/jtx/impl/sig.cpp index fa1977fe08..6ea1c153cb 100644 --- a/src/test/jtx/impl/sig.cpp +++ b/src/test/jtx/impl/sig.cpp @@ -29,12 +29,22 @@ sig::operator()(Env&, JTx& jt) const { if (!manual_) return; - jt.fill_sig = false; + if (!subField_) + jt.fill_sig = false; if (account_) { // VFALCO Inefficient pre-C++14 auto const account = *account_; - jt.signer = [account](Env&, JTx& jtx) { jtx::sign(jtx.jv, account); }; + auto callback = [subField = subField_, account](Env&, JTx& jtx) { + // Where to put the signature. Supports sfCounterPartySignature. + auto& sigObject = subField ? jtx[*subField] : jtx.jv; + + jtx::sign(jtx.jv, account, sigObject); + }; + if (!subField_) + jt.mainSigners.emplace_back(callback); + else + jt.postSigners.emplace_back(callback); } } diff --git a/src/test/jtx/impl/utility.cpp b/src/test/jtx/impl/utility.cpp index 27b45a32cb..fbdbaee4ef 100644 --- a/src/test/jtx/impl/utility.cpp +++ b/src/test/jtx/impl/utility.cpp @@ -44,14 +44,20 @@ parse(Json::Value const& jv) } void -sign(Json::Value& jv, Account const& account) +sign(Json::Value& jv, Account const& account, Json::Value& sigObject) { - jv[jss::SigningPubKey] = strHex(account.pk().slice()); + sigObject[jss::SigningPubKey] = strHex(account.pk().slice()); Serializer ss; ss.add32(HashPrefix::txSign); parse(jv).addWithoutSigningFields(ss); auto const sig = ripple::sign(account.pk(), account.sk(), ss.slice()); - jv[jss::TxnSignature] = strHex(Slice{sig.data(), sig.size()}); + sigObject[jss::TxnSignature] = strHex(Slice{sig.data(), sig.size()}); +} + +void +sign(Json::Value& jv, Account const& account) +{ + sign(jv, account, jv); } void diff --git a/src/test/jtx/mpt.h b/src/test/jtx/mpt.h index 2eacac68ec..422afa8fab 100644 --- a/src/test/jtx/mpt.h +++ b/src/test/jtx/mpt.h @@ -235,7 +235,7 @@ public: getBalance(Account const& account) const; MPT - operator[](std::string const& name); + operator[](std::string const& name) const; private: using SLEP = std::shared_ptr; diff --git a/src/test/jtx/multisign.h b/src/test/jtx/multisign.h index 1fed895c6d..7a035a9ff0 100644 --- a/src/test/jtx/multisign.h +++ b/src/test/jtx/multisign.h @@ -67,18 +67,63 @@ class msig { public: std::vector signers; + /** Alternative transaction object field in which to place the signer list. + * + * subField is only supported if an account_ is provided as well. + */ + SField const* const subField = nullptr; + /// Used solely as a convenience placeholder for ctors that do _not_ specify + /// a subfield. + static constexpr SField* const topLevel = nullptr; - msig(std::vector signers_) : signers(std::move(signers_)) + msig(SField const* subField_, std::vector signers_) + : signers(std::move(signers_)), subField(subField_) { sortSigners(signers); } + msig(SField const& subField_, std::vector signers_) + : msig{&subField_, signers_} + { + } + + msig(std::vector signers_) : msig(topLevel, signers_) + { + } + template requires std::convertible_to - explicit msig(AccountType&& a0, Accounts&&... aN) - : signers{std::forward(a0), std::forward(aN)...} + explicit msig(SField const* subField_, AccountType&& a0, Accounts&&... aN) + : msig{ + subField_, + std::vector{ + std::forward(a0), + std::forward(aN)...}} + { + } + + template + requires std::convertible_to + explicit msig(SField const& subField_, AccountType&& a0, Accounts&&... aN) + : msig{ + &subField_, + std::vector{ + std::forward(a0), + std::forward(aN)...}} + { + } + + template + requires( + std::convertible_to && + !std::is_same_v) + explicit msig(AccountType&& a0, Accounts&&... aN) + : msig{ + topLevel, + std::vector{ + std::forward(a0), + std::forward(aN)...}} { - sortSigners(signers); } void diff --git a/src/test/jtx/sig.h b/src/test/jtx/sig.h index aa65a4f697..b96a306a37 100644 --- a/src/test/jtx/sig.h +++ b/src/test/jtx/sig.h @@ -35,7 +35,20 @@ class sig { private: bool manual_ = true; + /** Alternative transaction object field in which to place the signature. + * + * subField is only supported if an account_ is provided as well. + */ + SField const* const subField_ = nullptr; + /** Account that will generate the signature. + * + * If not provided, no signature will be added by this helper. See also + * Env::autofill_sig. + */ std::optional account_; + /// Used solely as a convenience placeholder for ctors that do _not_ specify + /// a subfield. + static constexpr SField* const topLevel = nullptr; public: explicit sig(autofill_t) : manual_(false) @@ -46,7 +59,17 @@ public: { } - explicit sig(Account const& account) : account_(account) + explicit sig(SField const* subField, Account const& account) + : subField_(subField), account_(account) + { + } + + explicit sig(SField const& subField, Account const& account) + : sig(&subField, account) + { + } + + explicit sig(Account const& account) : sig(topLevel, account) { } diff --git a/src/test/jtx/utility.h b/src/test/jtx/utility.h index 2c21fbd3ff..9e89c3bb93 100644 --- a/src/test/jtx/utility.h +++ b/src/test/jtx/utility.h @@ -51,6 +51,12 @@ struct parse_error : std::logic_error STObject parse(Json::Value const& jv); +/** Sign automatically into a specific Json field of the jv object. + @note This only works on accounts with multi-signing off. +*/ +void +sign(Json::Value& jv, Account const& account, Json::Value& sigObject); + /** Sign automatically. @note This only works on accounts with multi-signing off. */ diff --git a/src/test/rpc/RPCCall_test.cpp b/src/test/rpc/RPCCall_test.cpp index d22896388d..26a6a536ef 100644 --- a/src/test/rpc/RPCCall_test.cpp +++ b/src/test/rpc/RPCCall_test.cpp @@ -4643,10 +4643,34 @@ static RPCCallTestData const rpcCallTestArray[] = { } ] })"}, - {"sign: too many arguments.", + {"sign: offline flag with signature_target.", __LINE__, {"sign", "my_secret", R"({"json_argument":true})", "offline", "extra"}, RPCCallTestData::no_exception, + R"({ + "method" : "sign", + "params" : [ + { + "api_version" : %API_VER%, + "offline" : true, + "secret" : "my_secret", + "signature_target" : "extra", + "tx_json" : + { + "json_argument" : true + } + } + ] + })"}, + {"sign: too many arguments.", + __LINE__, + {"sign", + "my_secret", + R"({"json_argument":true})", + "offline", + "CounterpartySignature", + "extra"}, + RPCCallTestData::no_exception, R"({ "method" : "sign", "params" : [ @@ -4675,20 +4699,24 @@ static RPCCallTestData const rpcCallTestArray[] = { } ] })"}, - {"sign: invalid final argument.", + {"sign: misspelled offline flag interpreted as signature_target.", __LINE__, {"sign", "my_secret", R"({"json_argument":true})", "offlin"}, RPCCallTestData::no_exception, R"({ - "method" : "sign", - "params" : [ - { - "error" : "invalidParams", - "error_code" : 31, - "error_message" : "Invalid parameters." - } - ] - })"}, + "method" : "sign", + "params" : [ + { + "api_version" : %API_VER%, + "secret" : "my_secret", + "signature_target" : "offlin", + "tx_json" : + { + "json_argument" : true + } + } + ] + })"}, // sign_for // -------------------------------------------------------------------- @@ -4880,10 +4908,34 @@ static RPCCallTestData const rpcCallTestArray[] = { } ] })"}, - {"submit: too many arguments.", + {"submit: offline flag with signature_target.", __LINE__, {"submit", "my_secret", R"({"json_argument":true})", "offline", "extra"}, RPCCallTestData::no_exception, + R"({ + "method" : "submit", + "params" : [ + { + "api_version" : %API_VER%, + "offline" : true, + "secret" : "my_secret", + "signature_target" : "extra", + "tx_json" : + { + "json_argument" : true + } + } + ] + })"}, + {"submit: too many arguments.", + __LINE__, + {"submit", + "my_secret", + R"({"json_argument":true})", + "offline", + "CounterpartySignature", + "extra"}, + RPCCallTestData::no_exception, R"({ "method" : "submit", "params" : [ @@ -4912,19 +4964,23 @@ static RPCCallTestData const rpcCallTestArray[] = { } ] })"}, - {"submit: last argument not \"offline\".", + {"submit: misspelled offline flag interpreted as signature_target.", __LINE__, {"submit", "my_secret", R"({"json_argument":true})", "offlne"}, RPCCallTestData::no_exception, R"({ - "method" : "submit", - "params" : [ - { - "error" : "invalidParams", - "error_code" : 31, - "error_message" : "Invalid parameters." - } - ] + "method" : "submit", + "params" : [ + { + "api_version" : %API_VER%, + "secret" : "my_secret", + "signature_target" : "offlne", + "tx_json" : + { + "json_argument" : true + } + } + ] })"}, // submit_multisigned diff --git a/src/xrpld/app/main/Main.cpp b/src/xrpld/app/main/Main.cpp index 2353d7acd1..3dff9d4d5f 100644 --- a/src/xrpld/app/main/Main.cpp +++ b/src/xrpld/app/main/Main.cpp @@ -173,7 +173,7 @@ printHelp(po::options_description const& desc) " server_state [counters]\n" " sign [offline]\n" " sign_for " - "[offline]\n" + "[offline] []\n" " stop\n" " simulate [|] []\n" " submit |[ ]\n" diff --git a/src/xrpld/app/tx/detail/Batch.cpp b/src/xrpld/app/tx/detail/Batch.cpp index cba89348d0..4ffdd2d57c 100644 --- a/src/xrpld/app/tx/detail/Batch.cpp +++ b/src/xrpld/app/tx/detail/Batch.cpp @@ -237,6 +237,39 @@ Batch::preflight(PreflightContext const& ctx) std::unordered_set uniqueHashes; std::unordered_map> accountSeqTicket; + auto checkSignatureFields = [&parentBatchId, &j = ctx.j]( + STObject const& sig, + uint256 const& hash, + char const* label = "") -> NotTEC { + if (sig.isFieldPresent(sfTxnSignature)) + { + JLOG(j.debug()) + << "BatchTrace[" << parentBatchId << "]: " + << "inner txn " << label << "cannot include TxnSignature. " + << "txID: " << hash; + return temBAD_SIGNATURE; + } + + if (sig.isFieldPresent(sfSigners)) + { + JLOG(j.debug()) + << "BatchTrace[" << parentBatchId << "]: " + << "inner txn " << label << " cannot include Signers. " + << "txID: " << hash; + return temBAD_SIGNER; + } + + if (!sig.getFieldVL(sfSigningPubKey).empty()) + { + JLOG(j.debug()) + << "BatchTrace[" << parentBatchId << "]: " + << "inner txn " << label << " SigningPubKey must be empty. " + << "txID: " << hash; + return temBAD_REGKEY; + } + + return tesSUCCESS; + }; for (STObject rb : rawTxns) { STTx const stx = STTx{std::move(rb)}; @@ -266,29 +299,23 @@ Batch::preflight(PreflightContext const& ctx) return temINVALID_FLAG; } - if (stx.isFieldPresent(sfTxnSignature)) - { - JLOG(ctx.j.debug()) << "BatchTrace[" << parentBatchId << "]: " - << "inner txn cannot include TxnSignature. " - << "txID: " << hash; - return temBAD_SIGNATURE; - } + if (auto const ret = checkSignatureFields(stx, hash)) + return ret; - if (stx.isFieldPresent(sfSigners)) + /* Placeholder for field that will be added by Lending Protocol + // Note that the CounterpartySignature is optional, and should not be + // included, but if it is, ensure it doesn't contain a signature. + if (stx.isFieldPresent(sfCounterpartySignature)) { - JLOG(ctx.j.debug()) << "BatchTrace[" << parentBatchId << "]: " - << "inner txn cannot include Signers. " - << "txID: " << hash; - return temBAD_SIGNER; - } - - if (!stx.getSigningPubKey().empty()) - { - JLOG(ctx.j.debug()) << "BatchTrace[" << parentBatchId << "]: " - << "inner txn SigningPubKey must be empty. " - << "txID: " << hash; - return temBAD_REGKEY; + auto const counterpartySignature = + stx.getFieldObject(sfCounterpartySignature); + if (auto const ret = checkSignatureFields( + counterpartySignature, hash, "counterparty signature ")) + { + return ret; + } } + */ auto const innerAccount = stx.getAccountID(sfAccount); if (auto const preflightResult = ripple::preflight( @@ -385,6 +412,13 @@ Batch::preflightSigValidated(PreflightContext const& ctx) // inner account to the required signers set. if (innerAccount != outerAccount) requiredSigners.insert(innerAccount); + /* Placeholder for field that will be added by Lending Protocol + // Some transactions have a Counterparty, who must also sign the + // transaction if they are not the outer account + if (auto const counterparty = rb.at(~sfCounterparty); + counterparty && counterparty != outerAccount) + requiredSigners.insert(*counterparty); + */ } // Validation Batch Signers diff --git a/src/xrpld/rpc/detail/RPCCall.cpp b/src/xrpld/rpc/detail/RPCCall.cpp index 57432d920f..822eb440b3 100644 --- a/src/xrpld/rpc/detail/RPCCall.cpp +++ b/src/xrpld/rpc/detail/RPCCall.cpp @@ -965,7 +965,16 @@ private: Json::Value txJSON; Json::Reader reader; bool const bOffline = - 3 == jvParams.size() && jvParams[2u].asString() == "offline"; + jvParams.size() >= 3 && jvParams[2u].asString() == "offline"; + std::optional const field = + [&jvParams, bOffline]() -> std::optional { + if (jvParams.size() < 3) + return std::nullopt; + if (jvParams.size() < 4 && bOffline) + return std::nullopt; + Json::UInt index = bOffline ? 3u : 2u; + return jvParams[index].asString(); + }(); if (1 == jvParams.size()) { @@ -978,7 +987,7 @@ private: return jvRequest; } else if ( - (2 == jvParams.size() || bOffline) && + (jvParams.size() >= 2 || bOffline) && reader.parse(jvParams[1u].asString(), txJSON)) { // Signing or submitting tx_json. @@ -990,6 +999,9 @@ private: if (bOffline) jvRequest[jss::offline] = true; + if (field) + jvRequest[jss::signature_target] = *field; + return jvRequest; } @@ -1270,11 +1282,11 @@ public: {"server_definitions", &RPCParser::parseServerDefinitions, 0, 1}, {"server_info", &RPCParser::parseServerInfo, 0, 1}, {"server_state", &RPCParser::parseServerInfo, 0, 1}, - {"sign", &RPCParser::parseSignSubmit, 2, 3}, + {"sign", &RPCParser::parseSignSubmit, 2, 4}, {"sign_for", &RPCParser::parseSignFor, 3, 4}, {"stop", &RPCParser::parseAsIs, 0, 0}, {"simulate", &RPCParser::parseSimulate, 1, 2}, - {"submit", &RPCParser::parseSignSubmit, 1, 3}, + {"submit", &RPCParser::parseSignSubmit, 1, 4}, {"submit_multisigned", &RPCParser::parseSubmitMultiSigned, 1, 1}, {"transaction_entry", &RPCParser::parseTransactionEntry, 2, 2}, {"tx", &RPCParser::parseTx, 1, 4}, diff --git a/src/xrpld/rpc/detail/TransactionSign.cpp b/src/xrpld/rpc/detail/TransactionSign.cpp index 175fd84c9b..aa7c706a19 100644 --- a/src/xrpld/rpc/detail/TransactionSign.cpp +++ b/src/xrpld/rpc/detail/TransactionSign.cpp @@ -33,6 +33,7 @@ #include #include #include +#include #include #include #include @@ -54,6 +55,7 @@ private: AccountID const* const multiSigningAcctID_; std::optional multiSignPublicKey_; Buffer multiSignature_; + std::optional> signatureTarget_; public: explicit SigningForParams() : multiSigningAcctID_(nullptr) @@ -116,12 +118,25 @@ public: return multiSignature_; } + std::optional> const& + getSignatureTarget() const + { + return signatureTarget_; + } + void setPublicKey(PublicKey const& multiSignPublicKey) { multiSignPublicKey_ = multiSignPublicKey; } + void + setSignatureTarget( + std::optional> const& field) + { + signatureTarget_ = field; + } + void moveMultiSignature(Buffer&& multiSignature) { @@ -427,6 +442,29 @@ transactionPreProcessImpl( bool const verify = !(params.isMember(jss::offline) && params[jss::offline].asBool()); + auto const signatureTarget = + [¶ms]() -> std::optional> { + if (params.isMember(jss::signature_target)) + return SField::getField(params[jss::signature_target].asString()); + return std::nullopt; + }(); + + // Make sure the signature target field is valid, if specified, and save the + // template for use later + auto const signatureTemplate = signatureTarget + ? InnerObjectFormats::getInstance().findSOTemplateBySField( + *signatureTarget) + : nullptr; + if (signatureTarget) + { + if (!signatureTemplate) + { // Invalid target field + return RPC::make_error( + rpcINVALID_PARAMS, signatureTarget->get().getName()); + } + signingArgs.setSignatureTarget(signatureTarget); + } + if (!params.isMember(jss::tx_json)) return RPC::missing_field_error(jss::tx_json); @@ -541,9 +579,10 @@ transactionPreProcessImpl( JLOG(j.trace()) << "verify: " << toBase58(calcAccountID(pk)) << " : " << toBase58(srcAddressID); - // Don't do this test if multisigning since the account and secret - // probably don't belong together in that case. - if (!signingArgs.isMultiSigning()) + // Don't do this test if multisigning or if the signature is going into + // an alternate field since the account and secret probably don't belong + // together in that case. + if (!signingArgs.isMultiSigning() && !signatureTarget) { // Make sure the account and secret belong together. if (tx_json.isMember(sfDelegate.jsonName)) @@ -598,7 +637,17 @@ transactionPreProcessImpl( { // If we're generating a multi-signature the SigningPubKey must be // empty, otherwise it must be the master account's public key. - parsed.object->setFieldVL( + STObject* sigObject = &*parsed.object; + if (signatureTarget) + { + // If the target object doesn't exist, make one. + if (!parsed.object->isFieldPresent(*signatureTarget)) + parsed.object->setFieldObject( + *signatureTarget, + STObject{*signatureTemplate, *signatureTarget}); + sigObject = &parsed.object->peekFieldObject(*signatureTarget); + } + sigObject->setFieldVL( sfSigningPubKey, signingArgs.isMultiSigning() ? Slice(nullptr, 0) : pk.slice()); @@ -630,7 +679,7 @@ transactionPreProcessImpl( } else if (signingArgs.isSingleSigning()) { - stTx->sign(pk, sk); + stTx->sign(pk, sk, signatureTarget); } return transactionPreProcessResult{std::move(stTx)}; @@ -1195,11 +1244,17 @@ transactionSignFor( signer.setFieldVL( sfSigningPubKey, signForParams.getPublicKey().slice()); + STObject& sigTarget = [&]() -> STObject& { + auto const target = signForParams.getSignatureTarget(); + if (target) + return sttx->peekFieldObject(*target); + return *sttx; + }(); // If there is not yet a Signers array, make one. - if (!sttx->isFieldPresent(sfSigners)) - sttx->setFieldArray(sfSigners, {}); + if (!sigTarget.isFieldPresent(sfSigners)) + sigTarget.setFieldArray(sfSigners, {}); - auto& signers = sttx->peekFieldArray(sfSigners); + auto& signers = sigTarget.peekFieldArray(sfSigners); signers.emplace_back(std::move(signer)); // The array must be sorted and validated.