diff --git a/src/ripple/app/misc/AmendmentTable.h b/src/ripple/app/misc/AmendmentTable.h index bba0840a5..c83f5277f 100644 --- a/src/ripple/app/misc/AmendmentTable.h +++ b/src/ripple/app/misc/AmendmentTable.h @@ -272,22 +272,25 @@ public: // Inject appropriate pseudo-transactions for (auto const& it : actions) { - STTx trans (ttAMENDMENT); - trans.setAccountID (sfAccount, AccountID()); - trans.setFieldH256 (sfAmendment, it.first); - trans.setFieldU32 (sfLedgerSequence, lastClosedLedger->seq() + 1); + STTx amendTx (ttAMENDMENT, + [&it, seq = lastClosedLedger->seq() + 1](auto& obj) + { + obj.setAccountID (sfAccount, AccountID()); + obj.setFieldH256 (sfAmendment, it.first); + obj.setFieldU32 (sfLedgerSequence, seq); - if (it.second != 0) - trans.setFieldU32 (sfFlags, it.second); + if (it.second != 0) + obj.setFieldU32 (sfFlags, it.second); + }); Serializer s; - trans.add (s); + amendTx.add (s); #if ! RIPPLE_PROPOSE_AMENDMENTS return; #endif - uint256 txID = trans.getTransactionID(); + uint256 txID = amendTx.getTransactionID(); auto tItem = std::make_shared (txID, s.peekData()); initialPosition->addGiveItem (tItem, true, false); } diff --git a/src/ripple/app/misc/FeeVoteImpl.cpp b/src/ripple/app/misc/FeeVoteImpl.cpp index 92ba91b75..49a6caaf7 100644 --- a/src/ripple/app/misc/FeeVoteImpl.cpp +++ b/src/ripple/app/misc/FeeVoteImpl.cpp @@ -201,6 +201,7 @@ FeeVoteImpl::doVoting (Ledger::ref lastClosedLedger, std::uint64_t const baseFee = baseFeeVote.getVotes (); std::uint32_t const baseReserve = baseReserveVote.getVotes (); std::uint32_t const incReserve = incReserveVote.getVotes (); + std::uint32_t const feeUnits = target_.reference_fee_units; // add transactions to our position if ((baseFee != lastClosedLedger->fees().base) || @@ -212,20 +213,23 @@ FeeVoteImpl::doVoting (Ledger::ref lastClosedLedger, "/" << baseReserve << "/" << incReserve; - STTx trans (ttFEE); - trans[sfAccount] = AccountID(); - trans[sfBaseFee] = baseFee; - trans[sfReferenceFeeUnits] = target_.reference_fee_units; - trans[sfReserveBase] = baseReserve; - trans[sfReserveIncrement] = incReserve; + STTx feeTx (ttFEE, + [baseFee,baseReserve,incReserve,feeUnits](auto& obj) + { + obj[sfAccount] = AccountID(); + obj[sfBaseFee] = baseFee; + obj[sfReserveBase] = baseReserve; + obj[sfReserveIncrement] = incReserve; + obj[sfReferenceFeeUnits] = feeUnits; + }); - uint256 txID = trans.getTransactionID (); + uint256 txID = feeTx.getTransactionID (); if (journal_.warning) journal_.warning << "Vote: " << txID; Serializer s; - trans.add (s); + feeTx.add (s); auto tItem = std::make_shared (txID, s.peekData ()); diff --git a/src/ripple/protocol/STTx.h b/src/ripple/protocol/STTx.h index d2fe8b85e..8b4fca626 100644 --- a/src/ripple/protocol/STTx.h +++ b/src/ripple/protocol/STTx.h @@ -24,7 +24,7 @@ #include #include #include -#include +#include namespace ripple { @@ -54,10 +54,19 @@ public: explicit STTx (SerialIter& sit); explicit STTx (SerialIter&& sit) : STTx(sit) {} - explicit STTx (TxType type); explicit STTx (STObject&& object); + /** Constructs a transaction. + + The returned transaction will have the specified type and + any fields that the callback function adds to the object + that's passed in. + */ + STTx ( + TxType type, + std::function assembler); + STBase* copy (std::size_t n, void* buf) const override { @@ -91,7 +100,6 @@ public: { return getFieldVL (sfSigningPubKey); } - void setSigningPubKey (const RippleAddress & naSignPubKey); std::uint32_t getSequence () const { @@ -105,7 +113,10 @@ public: boost::container::flat_set getMentionedAccounts() const; - uint256 getTransactionID () const; + uint256 getTransactionID () const + { + return tid_; + } Json::Value getJson (int options) const override; Json::Value getJson (int options, bool binary) const; @@ -132,7 +143,7 @@ private: bool checkSingleSign () const; bool checkMultiSign () const; - boost::optional tid_; + uint256 tid_; TxType tx_type_; }; diff --git a/src/ripple/protocol/impl/STTx.cpp b/src/ripple/protocol/impl/STTx.cpp index 6aa04a5cd..0272ec846 100644 --- a/src/ripple/protocol/impl/STTx.cpp +++ b/src/ripple/protocol/impl/STTx.cpp @@ -35,25 +35,25 @@ #include #include #include +#include #include namespace ripple { -STTx::STTx (TxType type) - : STObject (sfTransaction) - , tx_type_ (type) +static +auto getTxFormat (TxType type) { auto format = TxFormats::getInstance().findByType (type); if (format == nullptr) { - WriteLog (lsWARNING, STTx) << - "Transaction type: " << type; - Throw ("invalid transaction type"); + Throw ( + "Invalid transaction type " + + std::to_string ( + static_cast>(type))); } - set (format->elements); - setFieldU16 (sfTransactionType, format->getType ()); + return format; } STTx::STTx (STObject&& object) @@ -61,21 +61,8 @@ STTx::STTx (STObject&& object) { tx_type_ = static_cast (getFieldU16 (sfTransactionType)); - auto format = TxFormats::getInstance().findByType (tx_type_); - - if (!format) - { - WriteLog (lsWARNING, STTx) << - "Transaction type: " << tx_type_; - Throw ("invalid transaction type"); - } - - if (!setType (format->elements)) - { - WriteLog (lsWARNING, STTx) << - "Transaction not legal for format"; + if (!setType (getTxFormat (tx_type_)->elements)) Throw ("transaction not valid"); - } tid_ = getHash(HashPrefix::transactionID); } @@ -86,30 +73,33 @@ STTx::STTx (SerialIter& sit) int length = sit.getBytesLeft (); if ((length < Protocol::txMinSizeBytes) || (length > Protocol::txMaxSizeBytes)) - { - WriteLog (lsERROR, STTx) << - "Transaction has invalid length: " << length; Throw ("Transaction length invalid"); - } set (sit); tx_type_ = static_cast (getFieldU16 (sfTransactionType)); - auto format = TxFormats::getInstance().findByType (tx_type_); - - if (!format) - { - WriteLog (lsWARNING, STTx) << - "Invalid transaction type: " << tx_type_; - Throw ("invalid transaction type"); - } - - if (!setType (format->elements)) - { - WriteLog (lsWARNING, STTx) << - "Transaction not legal for format"; + if (!setType (getTxFormat (tx_type_)->elements)) Throw ("transaction not valid"); - } + + tid_ = getHash(HashPrefix::transactionID); +} + +STTx::STTx ( + TxType type, + std::function assembler) + : STObject (sfTransaction) +{ + auto format = getTxFormat (type); + + set (format->elements); + setFieldU16 (sfTransactionType, format->getType ()); + + assembler (*this); + + tx_type_ = static_cast(getFieldU16 (sfTransactionType)); + + if (tx_type_ != type) + LogicError ("Transaction type was mutated during assembly"); tid_ = getHash(HashPrefix::transactionID); } @@ -163,15 +153,6 @@ STTx::getSigningHash () const return STObject::getSigningHash (HashPrefix::txSign); } -uint256 -STTx::getTransactionID () const -{ - assert(tid_); - if (! tid_) - LogicError("digest is undefined"); - return *tid_; -} - Blob STTx::getSignature () const { try @@ -216,11 +197,6 @@ bool STTx::checkSign(bool allowMultiSign) const return sigGood; } -void STTx::setSigningPubKey (RippleAddress const& naSignPubKey) -{ - setFieldVL (sfSigningPubKey, naSignPubKey.getAccountPublic ()); -} - Json::Value STTx::getJson (int) const { Json::Value ret = STObject::getJson (0); diff --git a/src/ripple/protocol/tests/STTx.test.cpp b/src/ripple/protocol/tests/STTx.test.cpp index 129830d13..e1b1ee24a 100644 --- a/src/ripple/protocol/tests/STTx.test.cpp +++ b/src/ripple/protocol/tests/STTx.test.cpp @@ -38,10 +38,13 @@ public: RippleAddress publicAcct = RippleAddress::createAccountPublic (generator, 1); RippleAddress privateAcct = RippleAddress::createAccountPrivate (generator, seed, 1); - STTx j (ttACCOUNT_SET); - j.setAccountID (sfAccount, calcAccountID(publicAcct)); - j.setSigningPubKey (publicAcct); - j.setFieldVL (sfMessageKey, publicAcct.getAccountPublic ()); + STTx j (ttACCOUNT_SET, + [&publicAcct](auto& obj) + { + obj.setAccountID (sfAccount, calcAccountID(publicAcct)); + obj.setFieldVL (sfMessageKey, publicAcct.getAccountPublic ()); + obj.setFieldVL (sfSigningPubKey, publicAcct.getAccountPublic ()); + }); j.sign (privateAcct); unexpected (!j.checkSign (true), "Transaction fails signature test"); @@ -92,12 +95,14 @@ public: // VFALCO Use PublicKey here RippleAddress txnPublicAcct = txnSeed.createAccountPublic (txnGenerator, 1); - STTx txn (ttACCOUNT_SET); - txn.setAccountID (sfAccount, calcAccountID(txnPublicAcct)); - txn.setSigningPubKey (txnPublicAcct); - txn.setFieldVL (sfMessageKey, txnPublicAcct.getAccountPublic ()); - Blob const emptyBlob; // Make empty signature for multi-signing - txn.setFieldVL (sfSigningPubKey, emptyBlob); + STTx txn (ttACCOUNT_SET, + [&txnPublicAcct](auto& obj) + { + obj.setAccountID (sfAccount, calcAccountID(txnPublicAcct)); + obj.setFieldVL (sfMessageKey, txnPublicAcct.getAccountPublic ()); + // Make empty signature for multi-signing + obj.setFieldVL (sfSigningPubKey, {}); + }); // Create fields for a SigningAccount RippleAddress saSeed;