Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement new fields for the type and version of the transaction #1062

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/bench/verify_script.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
static CMutableTransaction BuildCreditingTransaction(const CScript& scriptPubKey)
{
CMutableTransaction txCredit;
txCredit.nVersion = 1;
txCredit.version = 1;
txCredit.nLockTime = 0;
txCredit.vin.resize(1);
txCredit.vout.resize(1);
Expand All @@ -35,7 +35,7 @@ static CMutableTransaction BuildCreditingTransaction(const CScript& scriptPubKey
static CMutableTransaction BuildSpendingTransaction(const CScript& scriptSig, const CMutableTransaction& txCredit)
{
CMutableTransaction txSpend;
txSpend.nVersion = 1;
txSpend.version = 1;
txSpend.nLockTime = 0;
txSpend.vin.resize(1);
txSpend.vout.resize(1);
Expand Down
4 changes: 2 additions & 2 deletions src/compressor.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ class CScriptCompressor
private:
/**
* make this static for now (there are only 6 special scripts defined)
* this can potentially be extended together with a new nVersion for
* transactions, in which case this value becomes dependent on nVersion
* this can potentially be extended together with a new version for
* transactions, in which case this value becomes dependent on version
* and nHeight of the enclosing transaction.
*/
static const unsigned int nSpecialScripts = 6;
Expand Down
7 changes: 3 additions & 4 deletions src/consensus/tx_verify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,10 @@ std::pair<int, int64_t> CalculateSequenceLocks(const CTransaction &tx, int flags
int nMinHeight = -1;
int64_t nMinTime = -1;

// tx.nVersion is signed integer so requires cast to unsigned otherwise
// we would be doing a signed comparison and half the range of nVersion
// tx.version is signed integer so requires cast to unsigned otherwise
// we would be doing a signed comparison and half the range of version
// wouldn't support BIP 68.
bool fEnforceBIP68 = static_cast<uint32_t>(tx.nVersion) >= 2
&& flags & LOCKTIME_VERIFY_SEQUENCE;
bool fEnforceBIP68 = tx.version >= 2 && flags & LOCKTIME_VERIFY_SEQUENCE;

// Do not enforce sequence numbers as a relative lock time
// unless we have been instructed to
Expand Down
4 changes: 1 addition & 3 deletions src/esperanza/walletextension.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -248,8 +248,7 @@ bool WalletExtension::SetMasterKeyFromSeed(const key::mnemonic::Seed &seed,
}

bool WalletExtension::BackupWallet() {
const std::string wallet_file_name = m_enclosing_wallet.GetName().empty() ?
"wallet.dat" : m_enclosing_wallet.GetName();
const std::string wallet_file_name = m_enclosing_wallet.GetName().empty() ? "wallet.dat" : m_enclosing_wallet.GetName();
const int64_t current_time = GetTime();

const std::string backup_wallet_filename =
Expand Down Expand Up @@ -612,7 +611,6 @@ void WalletExtension::VoteIfNeeded() {
return;
}


const CWalletTx *prev_tx = m_enclosing_wallet.GetWalletTx(validator->m_last_transaction_hash);
assert(prev_tx);

Expand Down
2 changes: 1 addition & 1 deletion src/policy/policy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ bool IsStandardTx(const CTransaction& tx, std::string& reason)
return true;
}

if (tx.nVersion > CTransaction::MAX_STANDARD_VERSION || tx.nVersion < 1) {
if (tx.version > CTransaction::MAX_STANDARD_VERSION || tx.version < 1) {
reason = "version";
return false;
}
Expand Down
4 changes: 2 additions & 2 deletions src/policy/policy.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ static const unsigned int DEFAULT_BLOCK_MAX_WEIGHT = MAX_BLOCK_WEIGHT - 4000;
static const unsigned int DEFAULT_BLOCK_MIN_TX_FEE = 1000;
/** The maximum weight for transactions we're willing to relay/mine */
static const unsigned int MAX_STANDARD_TX_WEIGHT = 400000;
/** The minimum non-witness size for transactions we're willing to relay/mine (1 segwit input + 1 P2WPKH output = 82 bytes) */
static const unsigned int MIN_STANDARD_TX_NONWITNESS_SIZE = 82;
/** The minimum non-witness size for transactions we're willing to relay/mine (1 segwit input + 1 P2WPKH output = 80 bytes) */
static const unsigned int MIN_STANDARD_TX_NONWITNESS_SIZE = 80;
/** Maximum number of signature check operations in an IsStandard() P2SH script */
static const unsigned int MAX_P2SH_SIGOPS = 15;
/** The maximum number of sigops we're willing to relay/mine in a single tx */
Expand Down
18 changes: 9 additions & 9 deletions src/primitives/transaction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,15 @@ std::string CTxOut::ToString() const
return strprintf("CTxOut(nValue=%d.%08d, scriptPubKey=%s)", nValue / UNIT, nValue % UNIT, HexStr(scriptPubKey).substr(0, 30));
}

CMutableTransaction::CMutableTransaction() : nVersion(CTransaction::CURRENT_VERSION), nLockTime(0) {}
CMutableTransaction::CMutableTransaction(const CTransaction& tx) : vin(tx.vin), vout(tx.vout), nVersion(tx.nVersion), nLockTime(tx.nLockTime) {}
CMutableTransaction::CMutableTransaction() : version(CTransaction::CURRENT_VERSION), type(TxType::REGULAR), nLockTime(0) {}
CMutableTransaction::CMutableTransaction(const CTransaction& tx) : vin(tx.vin), vout(tx.vout), version(tx.version), type(tx.type), nLockTime(tx.nLockTime) {}

void CMutableTransaction::SetType(TxType type) {
nVersion = (nVersion & 0x0000FFFF) | (static_cast<uint16_t>(type) << 16);
this->type = static_cast<uint8_t>(type);
}

void CMutableTransaction::SetVersion(uint16_t type) {
nVersion = (nVersion & 0xFFFF0000) | (static_cast<uint16_t>(type));
void CMutableTransaction::SetVersion(uint8_t version) {
this->version = version;
}

uint256 CMutableTransaction::GetHash() const
Expand All @@ -85,9 +85,9 @@ uint256 CTransaction::ComputeWitnessHash() const
}

/* For backward compatibility, the hash is initialized to 0. TODO: remove the need for this default constructor entirely. */
CTransaction::CTransaction() : vin(), vout(), nVersion(CTransaction::CURRENT_VERSION), nLockTime(0), hash{}, m_witness_hash{} {}
CTransaction::CTransaction(const CMutableTransaction& tx) : vin(tx.vin), vout(tx.vout), nVersion(tx.nVersion), nLockTime(tx.nLockTime), hash{ComputeHash()}, m_witness_hash{ComputeWitnessHash()} {}
CTransaction::CTransaction(CMutableTransaction&& tx) : vin(std::move(tx.vin)), vout(std::move(tx.vout)), nVersion(tx.nVersion), nLockTime(tx.nLockTime), hash{ComputeHash()}, m_witness_hash{ComputeWitnessHash()} {}
CTransaction::CTransaction() : vin(), vout(), version(CTransaction::CURRENT_VERSION), type(TxType::REGULAR), nLockTime(0), hash{}, m_witness_hash{} {}
CTransaction::CTransaction(const CMutableTransaction& tx) : vin(tx.vin), vout(tx.vout), version(tx.version), type(tx.type), nLockTime(tx.nLockTime), hash{ComputeHash()}, m_witness_hash{ComputeWitnessHash()} {}
CTransaction::CTransaction(CMutableTransaction&& tx) : vin(std::move(tx.vin)), vout(std::move(tx.vout)), version(tx.version), type(tx.type), nLockTime(tx.nLockTime), hash{ComputeHash()}, m_witness_hash{ComputeWitnessHash()} {}

CAmount CTransaction::GetValueOut() const
{
Expand All @@ -110,7 +110,7 @@ std::string CTransaction::ToString() const
std::string str;
str += strprintf("CTransaction(hash=%s, ver=%d, vin.size=%u, vout.size=%u, nLockTime=%u)\n",
GetHash().ToString().substr(0,10),
nVersion,
version,
vin.size(),
vout.size(),
nLockTime);
Expand Down
32 changes: 18 additions & 14 deletions src/primitives/transaction.h
Original file line number Diff line number Diff line change
Expand Up @@ -180,13 +180,15 @@ struct CMutableTransaction;

/**
* Basic transaction serialization format:
* - int32_t nVersion
* - uint8_t type
* - uint8_t version
* - std::vector<CTxIn> vin
* - std::vector<CTxOut> vout
* - uint32_t nLockTime
*
* Extended transaction serialization format:
* - int32_t nVersion
* - uint8_t type
* - uint8_t version
* - unsigned char dummy = 0x00
* - unsigned char flags (!= 0)
* - std::vector<CTxIn> vin
Expand All @@ -199,7 +201,8 @@ template<typename Stream, typename TxType>
inline void UnserializeTransaction(TxType& tx, Stream& s) {
const bool fAllowWitness = !(s.GetVersion() & SERIALIZE_TRANSACTION_NO_WITNESS);

s >> tx.nVersion;
s >> tx.type;
s >> tx.version;
unsigned char flags = 0;
tx.vin.clear();
tx.vout.clear();
Expand Down Expand Up @@ -234,7 +237,8 @@ template<typename Stream, typename TxType>
inline void SerializeTransaction(const TxType& tx, Stream& s) {
const bool fAllowWitness = !(s.GetVersion() & SERIALIZE_TRANSACTION_NO_WITNESS);

s << tx.nVersion;
s << tx.type;
s << tx.version;
unsigned char flags = 0;
// Consistency check
if (fAllowWitness) {
Expand Down Expand Up @@ -264,14 +268,12 @@ class TransactionBase
{
public:

//! Returns the version of this transaction (considers only the two lower bytes of nVersion).
uint16_t GetVersion() const {
return static_cast<uint16_t>(GetDerived()->nVersion);
uint8_t GetVersion() const {
return GetDerived()->version;
}

//! Returns the transaction type (TxType) of this transaction (stored in the two upper bytes of the nVersion field).
TxType GetType() const {
return TxType::_from_index_unchecked(GetDerived()->nVersion >> 16);
return TxType::_from_index_unchecked(GetDerived()->type);
}

bool IsRegular() const {
Expand Down Expand Up @@ -345,13 +347,13 @@ class CTransaction : public TransactionBase<CTransaction>
{
public:
// Default transaction version.
static const int32_t CURRENT_VERSION=2;
static const uint8_t CURRENT_VERSION=2;

// Changing the default transaction version requires a two step process: first
// adapting relay policy by bumping MAX_STANDARD_VERSION, and then later date
// bumping the default CURRENT_VERSION at which point both CURRENT_VERSION and
// MAX_STANDARD_VERSION will be equal.
static const int32_t MAX_STANDARD_VERSION=2;
static const uint8_t MAX_STANDARD_VERSION=2;

// The local variables are made const to prevent unintended modification
// without updating the cached hash value. However, CTransaction is not
Expand All @@ -369,7 +371,8 @@ class CTransaction : public TransactionBase<CTransaction>
* Thus the upper two bytes are used to piggy back that type (also uint16_t) onto
* the version field.
*/
const uint32_t nVersion;
const uint8_t version;
const uint8_t type;
const uint32_t nLockTime;

private:
Expand Down Expand Up @@ -435,7 +438,8 @@ struct CMutableTransaction : public TransactionBase<CMutableTransaction>
{
std::vector<CTxIn> vin;
std::vector<CTxOut> vout;
uint32_t nVersion;
uint8_t version;
uint8_t type;
uint32_t nLockTime;

CMutableTransaction();
Expand All @@ -457,7 +461,7 @@ struct CMutableTransaction : public TransactionBase<CMutableTransaction>
Unserialize(s);
}

void SetVersion(uint16_t version);
void SetVersion(uint8_t version);

void SetType(TxType type);

Expand Down
2 changes: 1 addition & 1 deletion src/proposer/proposer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ class ProposerImpl : public Proposer {
continue;
}
const EligibleCoin &coin = winning_ticket.get();
LogPrint(BCLog::PROPOSING, "Proposing... (wallet=%s, coin=%s)\n",
LogPrint(BCLog::PROPOSING, "Proposing... (wallet=%s, coin=%d)\n",
wallet_name, util::to_string(coin.utxo));
staking::TransactionPicker::PickTransactionsParameters parameters{};
staking::TransactionPicker::PickTransactionsResult result =
Expand Down
12 changes: 8 additions & 4 deletions src/script/interpreter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1333,8 +1333,10 @@ class CTransactionSignatureSerializer
/** Serialize txTo */
template<typename S>
void Serialize(S &s) const {
// Serialize nVersion
::Serialize(s, txTo.nVersion);
// Serialize type
::Serialize(s, txTo.type);
// Serialize version
::Serialize(s, txTo.version);
// Serialize vin
unsigned int nInputs = fAnyoneCanPay ? 1 : txTo.vin.size();
::WriteCompactSize(s, nInputs);
Expand Down Expand Up @@ -1433,8 +1435,10 @@ uint256 SignatureHash(const CScript& scriptCode, const T& txTo, unsigned int nIn
}

CHashWriter ss(SER_GETHASH, 0);
// Type
ss << txTo.type;
// Version
ss << txTo.nVersion;
ss << txTo.version;
// Input prevouts/nSequence (none/all, depending on flags)
ss << hashPrevouts;
ss << hashSequence;
Expand Down Expand Up @@ -1549,7 +1553,7 @@ bool GenericTransactionSignatureChecker<T>::CheckSequence(const CScriptNum& nSeq

// Fail if the transaction's version number is not set high
// enough to trigger BIP 68 rules.
if (static_cast<uint32_t>(txTo->nVersion) < 2)
if (txTo->version < 2)
return false;

// Sequence numbers with their most significant bit set are not
Expand Down
Loading