zecfoundation
/
zcashd
mirror of https://github.com/ZcashFoundation/zcashd.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

CTransaction: Make new ZIP 225 fields non-const and private 

Using `const_cast` to serialize into an otherwise-constant field is
undefined behaviour:

  https://github.com/zcash/zcash/issues/967#issuecomment-225467855

Instead, we should make CTransaction's members non-const and private,
and provide accessors. It's not practical to make this change everywhere
yet, but we can start by only introducing new fields in this way. We
will need to provide accessors for orchardBundle's properties in any
case, since we need to call across the Rust FFI.
This commit is contained in:
Jack Grigg 2021-06-12 00:11:06 +01:00
parent 36874e7f07
commit 1ef818103d
2 changed files with 19 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
src/primitives/transaction.cpp
View File

@ -127,10 +127,10 @@ std::string CTxOut::ToString() const
CMutableTransaction::CMutableTransaction() : nVersion(CTransaction::SPROUT_MIN_CURRENT_VERSION), fOverwintered(false), nVersionGroupId(0), nExpiryHeight(0), nLockTime(0), valueBalance(0) {}
CMutableTransaction::CMutableTransaction(const CTransaction& tx) : nVersion(tx.nVersion), fOverwintered(tx.fOverwintered), nVersionGroupId(tx.nVersionGroupId), nExpiryHeight(tx.nExpiryHeight),
nConsensusBranchId(tx.nConsensusBranchId),
nConsensusBranchId(tx.GetConsensusBranchId()),
vin(tx.vin), vout(tx.vout), nLockTime(tx.nLockTime),
valueBalance(tx.valueBalance), vShieldedSpend(tx.vShieldedSpend), vShieldedOutput(tx.vShieldedOutput),
orchardBundle(tx.orchardBundle),
orchardBundle(tx.GetOrchardBundle()),
vJoinSplit(tx.vJoinSplit), joinSplitPubKey(tx.joinSplitPubKey), joinSplitSig(tx.joinSplitSig),
bindingSig(tx.bindingSig)
{
@ -200,7 +200,7 @@ CTransaction& CTransaction::operator=(const CTransaction &tx) {
*const_cast<bool*>(&fOverwintered) = tx.fOverwintered;
*const_cast<int*>(&nVersion) = tx.nVersion;
*const_cast<uint32_t*>(&nVersionGroupId) = tx.nVersionGroupId;
*const_cast<uint32_t*>(&nConsensusBranchId) = tx.nConsensusBranchId;
nConsensusBranchId = tx.nConsensusBranchId;
*const_cast<std::vector<CTxIn>*>(&vin) = tx.vin;
*const_cast<std::vector<CTxOut>*>(&vout) = tx.vout;
*const_cast<unsigned int*>(&nLockTime) = tx.nLockTime;
@ -208,7 +208,7 @@ CTransaction& CTransaction::operator=(const CTransaction &tx) {
*const_cast<CAmount*>(&valueBalance) = tx.valueBalance;
*const_cast<std::vector<SpendDescription>*>(&vShieldedSpend) = tx.vShieldedSpend;
*const_cast<std::vector<OutputDescription>*>(&vShieldedOutput) = tx.vShieldedOutput;
*const_cast<OrchardBundle*>(&orchardBundle) = tx.orchardBundle;
orchardBundle = tx.orchardBundle;
*const_cast<std::vector<JSDescription>*>(&vJoinSplit) = tx.vJoinSplit;
*const_cast<Ed25519VerificationKey*>(&joinSplitPubKey) = tx.joinSplitPubKey;
*const_cast<Ed25519Signature*>(&joinSplitSig) = tx.joinSplitSig;

21
src/primitives/transaction.h
View File

@ -707,6 +707,11 @@ struct CMutableTransaction;
class CTransaction
{
private:
/// The consensus branch ID that this transaction commits to.
/// Serialized from v5 onwards.
uint32_t nConsensusBranchId;
OrchardBundle orchardBundle;
/** Memory only. */
const uint256 hash;
void UpdateHash() const;
@ -754,9 +759,6 @@ public:
const bool fOverwintered;
const int32_t nVersion;
const uint32_t nVersionGroupId;
/// The consensus branch ID that this transaction commits to.
/// Serialized from v5 onwards.
const uint32_t nConsensusBranchId;
const std::vector<CTxIn> vin;
const std::vector<CTxOut> vout;
const uint32_t nLockTime;
@ -764,7 +766,6 @@ public:
const CAmount valueBalance;
const std::vector<SpendDescription> vShieldedSpend;
const std::vector<OutputDescription> vShieldedOutput;
const OrchardBundle orchardBundle;
const std::vector<JSDescription> vJoinSplit;
const Ed25519VerificationKey joinSplitPubKey;
const Ed25519Signature joinSplitSig;
@ -828,7 +829,7 @@ public:
if (isZip225V5) {
// Common Transaction Fields (plus version bytes above)
READWRITE(*const_cast<uint32_t*>(&this->nConsensusBranchId));
READWRITE(nConsensusBranchId);
READWRITE(*const_cast<uint32_t*>(&nLockTime));
READWRITE(*const_cast<uint32_t*>(&nExpiryHeight));
@ -856,7 +857,7 @@ public:
}
// Orchard Transaction Fields
READWRITE(*const_cast<OrchardBundle*>(&orchardBundle));
READWRITE(orchardBundle);
} else {
// Legacy transaction formats
READWRITE(*const_cast<std::vector<CTxIn>*>(&vin));
@ -908,6 +909,14 @@ public:
return header;
}
uint32_t GetConsensusBranchId() const {
return nConsensusBranchId;
}
const OrchardBundle& GetOrchardBundle() const {
return orchardBundle;
}
/*
* Context for the two methods below:
* As at most one of vpub_new and vpub_old is non-zero in every JoinSplit,