refactor: Add support for extra transaction signature validation (#5851)

- Restructures `STTx` signature checking code to be able to handle
  a `sigObject`, which may be the full transaction, or may be an object
  field containing a separate signature. Either way, the `sigObject` can
  be a single- or multi-sign signature.
- This is distinct from 550f90a75e (#5594), which changed the check in
  Transactor, which validates whether a given account is allowed to sign
  for the given transaction. This cryptographically checks the signature
  validity.
This commit is contained in:
Ed Hennis
2025-10-30 15:39:56 -04:00
committed by GitHub
parent b39d7a6519
commit 8d1b3b3994
22 changed files with 511 additions and 130 deletions

View File

@@ -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 <class Tag>
void

View File

@@ -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<std::reference_wrapper<SField const>> 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<void, std::string>
checkSign(RequireFullyCanonicalSig requireCanonicalSig, Rules const& rules)
const;
@@ -150,17 +163,34 @@ public:
char status,
std::string const& escapedMetaData) const;
std::vector<uint256>
std::vector<uint256> 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<void, std::string>
checkSingleSign(RequireFullyCanonicalSig requireCanonicalSig) const;
checkSign(
RequireFullyCanonicalSig requireCanonicalSig,
Rules const& rules,
STObject const& sigObject) const;
Expected<void, std::string>
checkSingleSign(
RequireFullyCanonicalSig requireCanonicalSig,
STObject const& sigObject) const;
Expected<void, std::string>
checkMultiSign(
RequireFullyCanonicalSig requireCanonicalSig,
Rules const& rules) const;
Rules const& rules,
STObject const& sigObject) const;
Expected<void, std::string>
checkBatchSingleSign(
@@ -179,7 +209,7 @@ private:
move(std::size_t n, void* buf) override;
friend class detail::STVar;
mutable std::vector<uint256> batch_txn_ids_;
mutable std::vector<uint256> batchTxnIds_;
};
bool

View File

@@ -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

View File

@@ -688,6 +688,16 @@ STObject::getFieldV256(SField const& field) const
return getFieldByConstRef<STVector256>(field, empty);
}
STObject
STObject::getFieldObject(SField const& field) const
{
STObject const empty{field};
auto ret = getFieldByConstRef<STObject>(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
{

View File

@@ -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<std::reference_wrapper<SField const>> 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<void, std::string>
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<void, std::string>
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<void, std::string>
@@ -382,23 +415,23 @@ STTx::getMetaSQL(
static Expected<void, std::string>
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<void, std::string>
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<void, std::string>
@@ -440,31 +475,29 @@ STTx::checkBatchSingleSign(
Expected<void, std::string>
multiSignHelper(
STObject const& signerObj,
STObject const& sigObject,
std::optional<AccountID> txnAccountID,
bool const fullyCanonical,
std::function<Serializer(AccountID const&)> 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<std::string> 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<void, std::string>
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<AccountID>(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<uint256>
std::vector<uint256> 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_;
}
//------------------------------------------------------------------------------

View File

@@ -54,7 +54,11 @@ struct JTx
bool fill_sig = true;
bool fill_netid = true;
std::shared_ptr<STTx const> stx;
std::function<void(Env&, JTx&)> signer;
// Functions that sign the transaction from the Account
std::vector<std::function<void(Env&, JTx&)>> mainSigners;
// Functions that sign something else after the mainSigners, such as
// sfCounterpartySignature
std::vector<std::function<void(Env&, JTx&)>> postSigners;
JTx() = default;
JTx(JTx const&) = default;

View File

@@ -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 <class Suite>

View File

@@ -213,14 +213,16 @@ public:
template <std::integral T>
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, ""};
}

View File

@@ -34,6 +34,7 @@
#include <xrpl/basics/Slice.h>
#include <xrpl/basics/contract.h>
#include <xrpl/basics/scope.h>
#include <xrpl/json/to_string.h>
#include <xrpl/net/HTTPClient.h>
#include <xrpl/protocol/ErrorCodes.h>
@@ -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)

View File

@@ -507,7 +507,7 @@ MPTTester::getFlags(std::optional<Account> const& holder) const
}
MPT
MPTTester::operator[](std::string const& name)
MPTTester::operator[](std::string const& name) const
{
return MPT(name, issuanceID());
}

View File

@@ -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<STObject> 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

View File

@@ -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);
}
}

View File

@@ -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

View File

@@ -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<SLE const>;

View File

@@ -67,18 +67,63 @@ class msig
{
public:
std::vector<Reg> 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<Reg> signers_) : signers(std::move(signers_))
msig(SField const* subField_, std::vector<Reg> signers_)
: signers(std::move(signers_)), subField(subField_)
{
sortSigners(signers);
}
msig(SField const& subField_, std::vector<Reg> signers_)
: msig{&subField_, signers_}
{
}
msig(std::vector<Reg> signers_) : msig(topLevel, signers_)
{
}
template <class AccountType, class... Accounts>
requires std::convertible_to<AccountType, Reg>
explicit msig(AccountType&& a0, Accounts&&... aN)
: signers{std::forward<AccountType>(a0), std::forward<Accounts>(aN)...}
explicit msig(SField const* subField_, AccountType&& a0, Accounts&&... aN)
: msig{
subField_,
std::vector<Reg>{
std::forward<AccountType>(a0),
std::forward<Accounts>(aN)...}}
{
}
template <class AccountType, class... Accounts>
requires std::convertible_to<AccountType, Reg>
explicit msig(SField const& subField_, AccountType&& a0, Accounts&&... aN)
: msig{
&subField_,
std::vector<Reg>{
std::forward<AccountType>(a0),
std::forward<Accounts>(aN)...}}
{
}
template <class AccountType, class... Accounts>
requires(
std::convertible_to<AccountType, Reg> &&
!std::is_same_v<AccountType, SField*>)
explicit msig(AccountType&& a0, Accounts&&... aN)
: msig{
topLevel,
std::vector<Reg>{
std::forward<AccountType>(a0),
std::forward<Accounts>(aN)...}}
{
sortSigners(signers);
}
void

View File

@@ -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> 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)
{
}

View File

@@ -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.
*/

View File

@@ -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

View File

@@ -173,7 +173,7 @@ printHelp(po::options_description const& desc)
" server_state [counters]\n"
" sign <private_key> <tx_json> [offline]\n"
" sign_for <signer_address> <signer_private_key> <tx_json> "
"[offline]\n"
"[offline] [<signature_field>]\n"
" stop\n"
" simulate [<tx_blob>|<tx_json>] [<binary>]\n"
" submit <tx_blob>|[<private_key> <tx_json>]\n"

View File

@@ -237,6 +237,39 @@ Batch::preflight(PreflightContext const& ctx)
std::unordered_set<uint256> uniqueHashes;
std::unordered_map<AccountID, std::unordered_set<std::uint32_t>>
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

View File

@@ -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<std::string> const field =
[&jvParams, bOffline]() -> std::optional<std::string> {
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},

View File

@@ -33,6 +33,7 @@
#include <xrpl/basics/mulDiv.h>
#include <xrpl/json/json_writer.h>
#include <xrpl/protocol/ErrorCodes.h>
#include <xrpl/protocol/InnerObjectFormats.h>
#include <xrpl/protocol/RPCErr.h>
#include <xrpl/protocol/STParsedJSON.h>
#include <xrpl/protocol/Sign.h>
@@ -54,6 +55,7 @@ private:
AccountID const* const multiSigningAcctID_;
std::optional<PublicKey> multiSignPublicKey_;
Buffer multiSignature_;
std::optional<std::reference_wrapper<SField const>> signatureTarget_;
public:
explicit SigningForParams() : multiSigningAcctID_(nullptr)
@@ -116,12 +118,25 @@ public:
return multiSignature_;
}
std::optional<std::reference_wrapper<SField const>> const&
getSignatureTarget() const
{
return signatureTarget_;
}
void
setPublicKey(PublicKey const& multiSignPublicKey)
{
multiSignPublicKey_ = multiSignPublicKey;
}
void
setSignatureTarget(
std::optional<std::reference_wrapper<SField const>> 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 =
[&params]() -> std::optional<std::reference_wrapper<SField const>> {
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.