Skip to content

Commit

Permalink
Merge #2044: [Wallet] Making CWalletTx store TransactionRef.
Browse files Browse the repository at this point in the history
652981a Make CWalletTx store a CTransactionRef (furszy)
53746b9 Transaction primitive, decouple serialization process into its own method. (furszy)
008f38e CTxIn primitive, remove unused prevPubKey member. (furszy)

Pull request description:

  Instead of inheriting from `CTransaction`, now `CWalletTx` will store it as member. Another step forward over the transaction complete immutability goal.

  An adaptation of c3f5673 with further needed changes, essentially:

  > it makes it actually fully immutable and never modifies any of its elements (apart from wit) after construction.
  >
  > To do so, we heavily rely on C++11. CTransaction gets a (CMutableTransaction&&) constructor that efficiently converts a CMutableTransaction into a CTransaction without copying. In addition, CTransaction gets a deserializing constructors.
  >
  > One downside is that CWalletTx cannot easily inherent from CTransaction anymore, as CWalletTx needs a way to modify the CTransaction data inside. By turning the superclass into a CTransactionRef field, it can take advantage (not implemented yet) of sharing CTransactions with the mempool.

ACKs for top commit:
  random-zebra:
    utACK 652981a
  Fuzzbawls:
    utACK 652981a

Tree-SHA512: bec9fb42de0e9fa2fa98ca0edb750ca0932beb36b0103cdc36f9f4e3dc75f8d9d78dbc1459197e31159e1c6c0dc6a1b47aede426734b8f1e01c474b775264b66
  • Loading branch information
furszy committed Jan 29, 2021
2 parents 4491e3b + 652981a commit b7871cb
Show file tree
Hide file tree
Showing 16 changed files with 289 additions and 284 deletions.
72 changes: 36 additions & 36 deletions src/primitives/transaction.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,8 @@ class CTxIn
COutPoint prevout;
CScript scriptSig;
uint32_t nSequence;
CScript prevPubKey;

CTxIn()
{
nSequence = std::numeric_limits<unsigned int>::max();
}

CTxIn() { nSequence = std::numeric_limits<unsigned int>::max(); }
explicit CTxIn(COutPoint prevoutIn, CScript scriptSigIn=CScript(), uint32_t nSequenceIn=std::numeric_limits<unsigned int>::max());
CTxIn(uint256 hashPrevTx, uint32_t nOut, CScript scriptSigIn=CScript(), uint32_t nSequenceIn=std::numeric_limits<uint32_t>::max());

Expand Down Expand Up @@ -221,8 +216,33 @@ class CTxOut

struct CMutableTransaction;

/**
* Transaction serialization format:
* - int32_t nVersion
* - std::vector<CTxIn> vin
* - std::vector<CTxOut> vout
* - uint32_t nLockTime
* - Optional<SaplingTxData> sapData
* - Optional<std::vector<uint8_t>> extraPayload
*/
template<typename Stream, typename Operation, typename TxType>
inline void SerializeTransaction(TxType& tx, Stream& s, Operation ser_action) {
READWRITE(*const_cast<int16_t*>(&tx.nVersion));
READWRITE(*const_cast<int16_t*>(&tx.nType));
READWRITE(*const_cast<std::vector<CTxIn>*>(&tx.vin));
READWRITE(*const_cast<std::vector<CTxOut>*>(&tx.vout));
READWRITE(*const_cast<uint32_t*>(&tx.nLockTime));

if (g_IsSaplingActive && tx.isSaplingVersion()) {
READWRITE(*const_cast<Optional<SaplingTxData>*>(&tx.sapData));
if (!tx.IsNormalType()) {
READWRITE(*const_cast<Optional<std::vector<uint8_t>>*>(&tx.extraPayload));
}
}
}

/** The basic transaction that is broadcasted on the network and contained in
* blocks. A transaction can contain multiple inputs and outputs.
* blocks. A transaction can contain multiple inputs and outputs.
*/
class CTransaction
{
Expand Down Expand Up @@ -273,20 +293,10 @@ class CTransaction

template <typename Stream, typename Operation>
inline void SerializationOp(Stream& s, Operation ser_action) {
READWRITE(*const_cast<int16_t*>(&nVersion));
READWRITE(*const_cast<int16_t*>(&nType));
READWRITE(*const_cast<std::vector<CTxIn>*>(&vin));
READWRITE(*const_cast<std::vector<CTxOut>*>(&vout));
READWRITE(*const_cast<uint32_t*>(&nLockTime));

if (g_IsSaplingActive && isSaplingVersion()) {
READWRITE(*const_cast<Optional<SaplingTxData>*>(&sapData));
if (nType != TxType::NORMAL)
READWRITE(*const_cast<Optional<std::vector<uint8_t> >*>(&extraPayload));
}

if (ser_action.ForRead())
SerializeTransaction(*this, s, ser_action);
if (ser_action.ForRead()) {
UpdateHash();
}
}

template <typename Stream>
Expand Down Expand Up @@ -329,6 +339,8 @@ class CTransaction
return isSaplingVersion() && nType != TxType::NORMAL && hasExtraPayload();
}

bool IsNormalType() const { return nType == TxType::NORMAL; }

// Ensure that special and sapling fields are signed
SigVersion GetRequiredSigVersion() const
{
Expand Down Expand Up @@ -412,34 +424,22 @@ struct CMutableTransaction

template <typename Stream, typename Operation>
inline void SerializationOp(Stream& s, Operation ser_action) {
READWRITE(nVersion);
READWRITE(nType);
READWRITE(vin);
READWRITE(vout);
READWRITE(nLockTime);

if (g_IsSaplingActive && nVersion >= CTransaction::TxVersion::SAPLING) {
READWRITE(*const_cast<Optional<SaplingTxData>*>(&sapData));
if (nType != CTransaction::TxType::NORMAL)
READWRITE(*const_cast<Optional<std::vector<uint8_t> >*>(&extraPayload));
}
SerializeTransaction(*this, s, ser_action);
}

template <typename Stream>
CMutableTransaction(deserialize_type, Stream& s) {
Unserialize(s);
}

bool isSaplingVersion() const { return nVersion >= CTransaction::TxVersion::SAPLING; }
bool IsNormalType() const { return nType == CTransaction::TxType::NORMAL; }

/** Compute the hash of this CMutableTransaction. This is computed on the
* fly, as opposed to GetHash() in CTransaction, which uses a cached result.
*/
uint256 GetHash() const;

bool isSaplingVersion() const
{
return nVersion >= CTransaction::TxVersion::SAPLING;
}

// Ensure that special and sapling fields are signed
SigVersion GetRequiredSigVersion() const
{
Expand Down
2 changes: 1 addition & 1 deletion src/qt/pivx/coldstakingmodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ void ColdStakingModel::refresh()

const auto *wtx = utxo.tx;
const QString txId = QString::fromStdString(wtx->GetHash().GetHex());
const CTxOut& out = wtx->vout[utxo.i];
const CTxOut& out = wtx->tx->vout[utxo.i];

// First parse the cs delegation
CSDelegation delegation;
Expand Down
4 changes: 2 additions & 2 deletions src/qt/pivx/masternodewizarddialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -260,8 +260,8 @@ bool MasterNodeWizardDialog::createMN()
CWalletTx* walletTx = currentTransaction.getTransaction();
std::string txID = walletTx->GetHash().GetHex();
int indexOut = -1;
for (int i=0; i < (int)walletTx->vout.size(); i++) {
CTxOut& out = walletTx->vout[i];
for (int i=0; i < (int)walletTx->tx->vout.size(); i++) {
const CTxOut& out = walletTx->tx->vout[i];
if (out.nValue == 10000 * COIN) {
indexOut = i;
break;
Expand Down
28 changes: 14 additions & 14 deletions src/qt/pivx/sendconfirmdialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ TxDetailDialog::TxDetailDialog(QWidget *parent, bool _isConfirmDialog, const QSt

void TxDetailDialog::setInputsType(const CWalletTx* _tx)
{
if (_tx->sapData && _tx->sapData->vShieldedSpend.empty()) {
if (_tx->tx->sapData && _tx->tx->sapData->vShieldedSpend.empty()) {
ui->labelTitlePrevTx->setText(tr("Previous Transaction"));
ui->labelOutputIndex->setText(tr("Output Index"));
} else {
Expand All @@ -110,11 +110,11 @@ void TxDetailDialog::setData(WalletModel *_model, const QModelIndex &index)
ui->textId->setTextInteractionFlags(Qt::TextSelectableByMouse);
// future: subdivide shielded and transparent by type and
// do not show send xxx recipients for txes with a single output + change (show the address directly).
if (_tx->vout.size() == 1 || (_tx->sapData && _tx->sapData->vShieldedOutput.size() == 1)) {
if (_tx->tx->vout.size() == 1 || (_tx->tx->sapData && _tx->tx->sapData->vShieldedOutput.size() == 1)) {
ui->textSendLabel->setText((address.size() < 40) ? address : address.left(20) + "..." + address.right(20));
} else {
ui->textSendLabel->setText(QString::number(_tx->vout.size() +
(_tx->sapData ? _tx->sapData->vShieldedOutput.size() : 0)) + " recipients");
ui->textSendLabel->setText(QString::number(_tx->tx->vout.size() +
(_tx->tx->sapData ? _tx->tx->sapData->vShieldedOutput.size() : 0)) + " recipients");
}
ui->textSend->setVisible(false);
isShieldedToShieldedRecv = rec->type == TransactionRecord::Type::RecvWithShieldedAddress;
Expand All @@ -127,7 +127,7 @@ void TxDetailDialog::setData(WalletModel *_model, const QModelIndex &index)
}

setInputsType(_tx);
int inputsSize = (_tx->sapData && !_tx->sapData->vShieldedSpend.empty()) ? _tx->sapData->vShieldedSpend.size() : _tx->vin.size();
int inputsSize = (_tx->tx->sapData && !_tx->tx->sapData->vShieldedSpend.empty()) ? _tx->tx->sapData->vShieldedSpend.size() : _tx->tx->vin.size();
ui->textInputs->setText(QString::number(inputsSize) + shieldedInputsExtraMsg);
ui->textConfirmations->setText(QString::number(rec->status.depth));
ui->textDate->setText(GUIUtil::dateTimeStrWithSeconds(date));
Expand Down Expand Up @@ -226,7 +226,7 @@ void TxDetailDialog::setData(WalletModel *_model, WalletModelTransaction* _tx)
ui->labelDividerMemo->setVisible(false);
}

int inputsSize = (walletTx->sapData && !walletTx->sapData->vShieldedSpend.empty()) ? walletTx->sapData->vShieldedSpend.size() : walletTx->vin.size();
int inputsSize = (walletTx->tx->sapData && !walletTx->tx->sapData->vShieldedSpend.empty()) ? walletTx->tx->sapData->vShieldedSpend.size() : walletTx->tx->vin.size();
ui->textInputs->setText(QString::number(inputsSize));
ui->textFee->setText(BitcoinUnits::formatWithUnit(nDisplayUnit, txFee, false, BitcoinUnits::separatorAlways));
}
Expand Down Expand Up @@ -262,21 +262,21 @@ void TxDetailDialog::onInputsClicked()
if (showGrid) {
const CWalletTx* walletTx = (this->tx) ? this->tx->getTransaction() : model->getTx(this->txHash);
if (walletTx) {
if (walletTx->sapData && walletTx->sapData->vShieldedSpend.empty()) {
if (walletTx->tx->sapData && walletTx->tx->sapData->vShieldedSpend.empty()) {
// transparent inputs
ui->gridInputs->setMinimumHeight(50 + (50 * walletTx->vin.size()));
ui->gridInputs->setMinimumHeight(50 + (50 * walletTx->tx->vin.size()));
int i = 1;
for (const CTxIn& in : walletTx->vin) {
for (const CTxIn& in : walletTx->tx->vin) {
QString hash = QString::fromStdString(in.prevout.hash.GetHex());
loadInputs(hash.left(18) + "..." + hash.right(18),
QString::number(in.prevout.n),
ui->gridLayoutInput, i);
i++;
}
} else {
ui->gridInputs->setMinimumHeight(50 + (50 * walletTx->sapData->vShieldedSpend.size()));
ui->gridInputs->setMinimumHeight(50 + (50 * walletTx->tx->sapData->vShieldedSpend.size()));
bool fInfoAvailable = false;
for (int i = 0; i < (int) walletTx->sapData->vShieldedSpend.size(); ++i) {
for (int i = 0; i < (int) walletTx->tx->sapData->vShieldedSpend.size(); ++i) {
Optional<QString> opAddr = model->getShieldedAddressFromSpendDesc(walletTx, i);
if (opAddr) {
QString addr = *opAddr;
Expand Down Expand Up @@ -338,7 +338,7 @@ void TxDetailDialog::onOutputsClicked()

// transparent recipients
int i = 0;
for (const CTxOut& out : walletTx->vout) {
for (const CTxOut& out : walletTx->tx->vout) {
QString labelRes;
CTxDestination dest;
bool isCsAddress = out.scriptPubKey.IsPayToColdStaking();
Expand All @@ -354,8 +354,8 @@ void TxDetailDialog::onOutputsClicked()
}

// shielded recipients
if (walletTx->sapData) {
for (int j = 0; j < (int) walletTx->sapData->vShieldedOutput.size(); ++j) {
if (walletTx->tx->sapData) {
for (int j = 0; j < (int) walletTx->tx->sapData->vShieldedOutput.size(); ++j) {
const SaplingOutPoint op(walletTx->GetHash(), j);
// TODO: This only works for txs that are stored, not for when this is a confirmation dialog..
if (walletTx->mapSaplingNoteData.find(op) == walletTx->mapSaplingNoteData.end()) {
Expand Down
Loading

0 comments on commit b7871cb

Please sign in to comment.