coins: pack `Coin` height/coinbase consistently
What the developer wrote
Authored by Lőrinc
coins: pack `Coin` height/coinbase consistently
Serialize `Coin` metadata using the canonical (height << 1) | coinbase packing across `Coin` serialization, undo records, and coinstats hashing.
Cast the 31-bit `nHeight` bitfield to `uint32_t` before shifting to avoid signed promotion undefined behaviour.
What changed, and why it matters
This commit tidies up how Bitcoin Core packs two pieces of coin metadata—block height and whether the coin came from a coinbase transaction—into a single serialized number. It makes three different code paths use the exact same bit-packing formula and explicitly converts a signed height value to unsigned before shifting, which removes a technical C++ undefined-behavior risk. The actual serialized bytes do not change for normal values, so this is primarily a code-correctness and consistency fix rather than a fix for an active exploit.
Treat as a low-risk hardening/cleanup commit. Reviewers should verify that the new expression is bit-identical to the old one for the full range of nHeight (0..2^31-1) and that the undo compatibility branch (nHeight > 0) remains intact. No urgent deployment action is warranted.
Security signals we found
Undefined behavior removed: explicit uint32_t cast before left shift of signed bitfield
Serialization consistency: three independent code paths now use the same packing formula
No format change: old and new expressions are equivalent for all valid nHeight values
Comment-only documentation update in src/coins.h to match code
Evidence from the diff
The patch unifies Coin metadata serialization to the canonical (height << 1) | coinbase packing in Coin::Serialize, coinstats hashing (TxOutSer), and undo records (TxInUndoFormatter). It also casts the 31-bit nHeight bitfield to uint32_t before the left shift to avoid signed-integer promotion undefined behavior. For all valid heights the old and new expressions produce identical uint32_t values, so wire/DB formats are unchanged. The main security-relevant change is eliminating UB on a signed 31-bit bitfield shift.
Changed components
src/coins.h - Coin::Serializesrc/kernel/coinstats.cpp - coinstats hash serializationsrc/undo.h - TxInUndoFormatter undo record serializationInspect captured patch +5 / −4
diff --git a/src/coins.h b/src/coins.h
index e58586dd..bfd6c5b0 100644
--- a/src/coins.h
+++ b/src/coins.h
@@ -27,7 +27,7 @@
* A UTXO entry.
*
* Serialized format:
- * - VARINT((coinbase ? 1 : 0) | (height << 1))
+ * - VARINT((height << 1) | (coinbase ? 1 : 0))
* - the non-spent CTxOut (via TxOutCompression)
*/
class Coin
@@ -62,7 +62,7 @@ public:
template<typename Stream>
void Serialize(Stream &s) const {
assert(!IsSpent());
- uint32_t code = nHeight * uint32_t{2} + fCoinBase;
+ uint32_t code{(uint32_t{nHeight} << 1) | uint32_t{fCoinBase}};
::Serialize(s, VARINT(code));
::Serialize(s, Using<TxOutCompression>(out));
}
diff --git a/src/kernel/coinstats.cpp b/src/kernel/coinstats.cpp
index d287ec4b..53039e57 100644
--- a/src/kernel/coinstats.cpp
+++ b/src/kernel/coinstats.cpp
@@ -47,7 +47,7 @@ template <typename T>
static void TxOutSer(T& ss, const COutPoint& outpoint, const Coin& coin)
{
ss << outpoint;
- ss << static_cast<uint32_t>((coin.nHeight << 1) + coin.fCoinBase);
+ ss << ((uint32_t{coin.nHeight} << 1) | uint32_t{coin.fCoinBase});
ss << coin.out;
}
diff --git a/src/undo.h b/src/undo.h
index 5591fe6c..13b92349 100644
--- a/src/undo.h
+++ b/src/undo.h
@@ -23,7 +23,8 @@ struct TxInUndoFormatter
{
template<typename Stream>
void Ser(Stream &s, const Coin& txout) {
- ::Serialize(s, VARINT(txout.nHeight * uint32_t{2} + txout.fCoinBase ));
+ uint32_t nCode{(uint32_t{txout.nHeight} << 1) | uint32_t{txout.fCoinBase}};
+ ::Serialize(s, VARINT(nCode));
if (txout.nHeight > 0) {
// Required to maintain compatibility with older undo format.
::Serialize(s, (unsigned char)0);
Why this scored 24/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.