Knew from before that there are multiple places using these types of expressions involving WITNESS_SCALE_FACTOR. Here is a somewhat minimal diff that applies to those places. It’s sad that we allow expressing negative transaction sizes and weights in many places even after this diff… but I agree with deferring fixing that.
0diff --git a/src/consensus/consensus.h b/src/consensus/consensus.h
1index 71b5fe2468..bcfc1a0140 100644
2--- a/src/consensus/consensus.h
3+++ b/src/consensus/consensus.h
4@@ -18,7 +18,7 @@ static const int64_t MAX_BLOCK_SIGOPS_COST = 80000;
5 /** Coinbase transaction outputs can only be spent after this number of new blocks (network rule) */
6 static const int COINBASE_MATURITY = 100;
7
8-static const int WITNESS_SCALE_FACTOR = 4;
9+constexpr unsigned int WITNESS_SCALE_FACTOR{4};
10
11 static const size_t MIN_TRANSACTION_WEIGHT = WITNESS_SCALE_FACTOR * 60; // 60 is the lower bound for the size of a valid serialized CTransaction
12 static const size_t MIN_SERIALIZABLE_TRANSACTION_WEIGHT = WITNESS_SCALE_FACTOR * 10; // 10 is the lower bound for the size of a serialized CTransaction
13diff --git a/src/consensus/validation.h b/src/consensus/validation.h
14index 37a40e767e..5cf0b45c86 100644
15--- a/src/consensus/validation.h
16+++ b/src/consensus/validation.h
17@@ -129,15 +129,15 @@ class BlockValidationState : public ValidationState<BlockValidationResult> {};
18 // using only serialization with and without witness data. As witness_size
19 // is equal to total_size - stripped_size, this formula is identical to:
20 // weight = (stripped_size * 3) + total_size.
21-static inline int32_t GetTransactionWeight(const CTransaction& tx)
22+static inline uint32_t GetTransactionWeight(const CTransaction& tx)
23 {
24 return ::GetSerializeSize(TX_NO_WITNESS(tx)) * (WITNESS_SCALE_FACTOR - 1) + ::GetSerializeSize(TX_WITH_WITNESS(tx));
25 }
26-static inline int64_t GetBlockWeight(const CBlock& block)
27+static inline uint64_t GetBlockWeight(const CBlock& block)
28 {
29 return ::GetSerializeSize(TX_NO_WITNESS(block)) * (WITNESS_SCALE_FACTOR - 1) + ::GetSerializeSize(TX_WITH_WITNESS(block));
30 }
31-static inline int64_t GetTransactionInputWeight(const CTxIn& txin)
32+static inline uint64_t GetTransactionInputWeight(const CTxIn& txin)
33 {
34 // scriptWitness size is added here because witnesses and txins are split up in segwit serialization.
35 return ::GetSerializeSize(TX_NO_WITNESS(txin)) * (WITNESS_SCALE_FACTOR - 1) + ::GetSerializeSize(TX_WITH_WITNESS(txin)) + ::GetSerializeSize(txin.scriptWitness.stack);
36diff --git a/src/core_io.cpp b/src/core_io.cpp
37index 7492e9ca50..fdc9e2cbe7 100644
38--- a/src/core_io.cpp
39+++ b/src/core_io.cpp
40@@ -28,6 +28,7 @@
41 #include <undo.h>
42 #include <univalue.h>
43 #include <util/check.h>
44+#include <util/overflow.h>
45 #include <util/result.h>
46 #include <util/strencodings.h>
47 #include <util/string.h>
48@@ -435,7 +436,7 @@ void TxToUniv(const CTransaction& tx, const uint256& block_hash, UniValue& entry
49 entry.pushKV("hash", tx.GetWitnessHash().GetHex());
50 entry.pushKV("version", tx.version);
51 entry.pushKV("size", tx.ComputeTotalSize());
52- entry.pushKV("vsize", (GetTransactionWeight(tx) + WITNESS_SCALE_FACTOR - 1) / WITNESS_SCALE_FACTOR);
53+ entry.pushKV("vsize", CeilDiv(GetTransactionWeight(tx), WITNESS_SCALE_FACTOR));
54 entry.pushKV("weight", GetTransactionWeight(tx));
55 entry.pushKV("locktime", tx.nLockTime);
56
57diff --git a/src/kernel/mempool_entry.h b/src/kernel/mempool_entry.h
58index 58824f7a80..cc3e64fa02 100644
59--- a/src/kernel/mempool_entry.h
60+++ b/src/kernel/mempool_entry.h
61@@ -91,7 +91,7 @@ public:
62 : TxGraph::Ref(std::move(ref)),
63 tx{tx},
64 nFee{fee},
65- nTxWeight{GetTransactionWeight(*tx)},
66+ nTxWeight{static_cast<int32_t>(GetTransactionWeight(*tx))},
67 nUsageSize{RecursiveDynamicUsage(tx)},
68 nTime{time},
69 entry_sequence{entry_sequence},
70diff --git a/src/policy/policy.cpp b/src/policy/policy.cpp
71index 46ec238c3b..4d20fe7c1e 100644
72--- a/src/policy/policy.cpp
73+++ b/src/policy/policy.cpp
74@@ -373,22 +373,22 @@ bool SpendsNonAnchorWitnessProg(const CTransaction& tx, const CCoinsViewCache& p
75 return false;
76 }
77
78-int64_t GetSigOpsAdjustedWeight(int64_t weight, int64_t sigop_cost, unsigned int bytes_per_sigop)
79+uint64_t GetSigOpsAdjustedWeight(uint64_t weight, uint64_t sigop_cost, unsigned int bytes_per_sigop)
80 {
81 return std::max(weight, sigop_cost * bytes_per_sigop);
82 }
83
84-int64_t GetVirtualTransactionSize(int64_t nWeight, int64_t nSigOpCost, unsigned int bytes_per_sigop)
85+uint64_t GetVirtualTransactionSize(uint64_t weight, uint64_t sigop_cost, unsigned int bytes_per_sigop)
86 {
87- return (GetSigOpsAdjustedWeight(nWeight, nSigOpCost, bytes_per_sigop) + WITNESS_SCALE_FACTOR - 1) / WITNESS_SCALE_FACTOR;
88+ return CeilDiv(GetSigOpsAdjustedWeight(weight, sigop_cost, bytes_per_sigop), WITNESS_SCALE_FACTOR);
89 }
90
91-int64_t GetVirtualTransactionSize(const CTransaction& tx, int64_t nSigOpCost, unsigned int bytes_per_sigop)
92+uint64_t GetVirtualTransactionSize(const CTransaction& tx, uint64_t sigop_cost, unsigned int bytes_per_sigop)
93 {
94- return GetVirtualTransactionSize(GetTransactionWeight(tx), nSigOpCost, bytes_per_sigop);
95+ return GetVirtualTransactionSize(GetTransactionWeight(tx), sigop_cost, bytes_per_sigop);
96 }
97
98-int64_t GetVirtualTransactionInputSize(const CTxIn& txin, int64_t nSigOpCost, unsigned int bytes_per_sigop)
99+uint64_t GetVirtualTransactionInputSize(const CTxIn& txin, uint64_t sigop_cost, unsigned int bytes_per_sigop)
100 {
101- return GetVirtualTransactionSize(GetTransactionInputWeight(txin), nSigOpCost, bytes_per_sigop);
102+ return GetVirtualTransactionSize(GetTransactionInputWeight(txin), sigop_cost, bytes_per_sigop);
103 }
104diff --git a/src/policy/policy.h b/src/policy/policy.h
105index 35189d7133..5f7877ba75 100644
106--- a/src/policy/policy.h
107+++ b/src/policy/policy.h
108@@ -12,6 +12,7 @@
109 #include <script/interpreter.h>
110 #include <script/solver.h>
111 #include <util/feefrac.h>
112+#include <util/overflow.h>
113
114 #include <cstdint>
115 #include <string>
116@@ -175,9 +176,9 @@ bool IsWitnessStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs)
117 bool SpendsNonAnchorWitnessProg(const CTransaction& tx, const CCoinsViewCache& prevouts);
118
119 /** Compute the virtual transaction size (weight reinterpreted as bytes). */
120-int64_t GetVirtualTransactionSize(int64_t nWeight, int64_t nSigOpCost, unsigned int bytes_per_sigop);
121-int64_t GetVirtualTransactionSize(const CTransaction& tx, int64_t nSigOpCost, unsigned int bytes_per_sigop);
122-int64_t GetVirtualTransactionInputSize(const CTxIn& tx, int64_t nSigOpCost, unsigned int bytes_per_sigop);
123+uint64_t GetVirtualTransactionSize(uint64_t weight, uint64_t sigop_cost, unsigned int bytes_per_sigop);
124+uint64_t GetVirtualTransactionSize(const CTransaction& tx, uint64_t sigop_cost, unsigned int bytes_per_sigop);
125+uint64_t GetVirtualTransactionInputSize(const CTxIn& tx, uint64_t sigop_cost, unsigned int bytes_per_sigop);
126
127 static inline int64_t GetVirtualTransactionSize(const CTransaction& tx)
128 {
129@@ -189,8 +190,8 @@ static inline int64_t GetVirtualTransactionInputSize(const CTxIn& tx)
130 return GetVirtualTransactionInputSize(tx, 0, 0);
131 }
132
133-int64_t GetSigOpsAdjustedWeight(int64_t weight, int64_t sigop_cost, unsigned int bytes_per_sigop);
134+uint64_t GetSigOpsAdjustedWeight(uint64_t weight, uint64_t sigop_cost, unsigned int bytes_per_sigop);
135
136-static inline FeePerVSize ToFeePerVSize(FeePerWeight feerate) { return {feerate.fee, (feerate.size + WITNESS_SCALE_FACTOR - 1) / WITNESS_SCALE_FACTOR}; }
137+static inline FeePerVSize ToFeePerVSize(FeePerWeight feerate) { return {feerate.fee, static_cast<int32_t>(CeilDiv(static_cast<uint32_t>(feerate.size), WITNESS_SCALE_FACTOR))}; }
138
139 #endif // BITCOIN_POLICY_POLICY_H
140diff --git a/src/test/util/transaction_utils.cpp b/src/test/util/transaction_utils.cpp
141index 7b1e2eb3ed..f69207774f 100644
142--- a/src/test/util/transaction_utils.cpp
143+++ b/src/test/util/transaction_utils.cpp
144@@ -6,6 +6,7 @@
145 #include <consensus/validation.h>
146 #include <script/signingprovider.h>
147 #include <test/util/transaction_utils.h>
148+#include <util/overflow.h>
149
150 CMutableTransaction BuildCreditingTransaction(const CScript& scriptPubKey, int nValue)
151 {
152@@ -71,14 +72,14 @@ std::vector<CMutableTransaction> SetupDummyInputs(FillableSigningProvider& keyst
153 return dummyTransactions;
154 }
155
156-void BulkTransaction(CMutableTransaction& tx, int32_t target_weight)
157+void BulkTransaction(CMutableTransaction& tx, uint32_t target_weight)
158 {
159 tx.vout.emplace_back(0, CScript() << OP_RETURN);
160 auto unpadded_weight{GetTransactionWeight(CTransaction(tx))};
161 assert(target_weight >= unpadded_weight);
162
163 // determine number of needed padding bytes by converting weight difference to vbytes
164- auto dummy_vbytes = (target_weight - unpadded_weight + (WITNESS_SCALE_FACTOR - 1)) / WITNESS_SCALE_FACTOR;
165+ auto dummy_vbytes = CeilDiv(target_weight - unpadded_weight, WITNESS_SCALE_FACTOR);
166 // compensate for the increase of the compact-size encoded script length
167 // (note that the length encoding of the unpadded output script needs one byte)
168 dummy_vbytes -= GetSizeOfCompactSize(dummy_vbytes) - 1;
169diff --git a/src/test/util/transaction_utils.h b/src/test/util/transaction_utils.h
170index 867554f805..1b3df73e3d 100644
171--- a/src/test/util/transaction_utils.h
172+++ b/src/test/util/transaction_utils.h
173@@ -29,7 +29,7 @@ std::vector<CMutableTransaction> SetupDummyInputs(FillableSigningProvider& keyst
174
175 // bulk transaction to reach a certain target weight,
176 // by appending a single output with padded output script
177-void BulkTransaction(CMutableTransaction& tx, int32_t target_weight);
178+void BulkTransaction(CMutableTransaction& tx, uint32_t target_weight);
179
180 /**
181 * Produce a satisfying script (scriptSig or witness).
182diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
183index edde923452..656f3e676d 100644
184--- a/src/wallet/spend.cpp
185+++ b/src/wallet/spend.cpp
186@@ -167,7 +167,7 @@ TxSize CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *walle
187 }
188
189 // It's ok to use 0 as the number of sigops since we never create any pathological transaction.
190- return TxSize{GetVirtualTransactionSize(weight, 0, 0), weight};
191+ return TxSize{static_cast<int64_t>(GetVirtualTransactionSize(weight, 0, 0)), weight};
192 }
193
194 TxSize CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, const CCoinControl* coin_control)
