diff --git a/src/chain.h b/src/chain.h --- a/src/chain.h +++ b/src/chain.h @@ -295,14 +295,14 @@ inline void SerializationOp(Stream &s, Operation ser_action) { int _nVersion = s.GetVersion(); if (!(s.GetType() & SER_GETHASH)) { - READWRITE(VARINT(_nVersion)); + READWRITE(VARINT(_nVersion, VarIntMode::NONNEGATIVE_SIGNED)); } - READWRITE(VARINT(nHeight)); + READWRITE(VARINT(nHeight, VarIntMode::NONNEGATIVE_SIGNED)); READWRITE(nStatus); READWRITE(VARINT(nTx)); if (nStatus.hasData() || nStatus.hasUndo()) { - READWRITE(VARINT(nFile)); + READWRITE(VARINT(nFile, VarIntMode::NONNEGATIVE_SIGNED)); } if (nStatus.hasData()) { READWRITE(VARINT(nDataPos)); diff --git a/src/flatfile.h b/src/flatfile.h --- a/src/flatfile.h +++ b/src/flatfile.h @@ -19,7 +19,7 @@ template inline void SerializationOp(Stream &s, Operation ser_action) { - READWRITE(VARINT(nFile)); + READWRITE(VARINT(nFile, VarIntMode::NONNEGATIVE_SIGNED)); READWRITE(VARINT(nPos)); } diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -961,7 +961,8 @@ for (const auto &output : outputs) { ss << VARINT(output.first + 1); ss << output.second.GetTxOut().scriptPubKey; - ss << VARINT(output.second.GetTxOut().nValue / SATOSHI); + ss << VARINT(output.second.GetTxOut().nValue / SATOSHI, + VarIntMode::NONNEGATIVE_SIGNED); stats.nTransactionOutputs++; stats.nTotalAmount += output.second.GetTxOut().nValue; stats.nBogoSize += @@ -969,7 +970,7 @@ 8 /* amount */ + 2 /* scriptPubKey len */ + output.second.GetTxOut().scriptPubKey.size() /* scriptPubKey */; } - ss << VARINT(0); + ss << VARINT(0u); } //! Calculate statistics about the unspent transaction output set diff --git a/src/serialize.h b/src/serialize.h --- a/src/serialize.h +++ b/src/serialize.h @@ -350,7 +350,32 @@ * 255: [0x80 0x7F] 65535: [0x82 0xFE 0x7F] * 2^32: [0x8E 0xFE 0xFE 0xFF 0x00] */ -template inline unsigned int GetSizeOfVarInt(I n) { + +/** + * Mode for encoding VarInts. + * + * Currently there is no support for signed encodings. The default mode will not + * compile with signed values, and the legacy "nonnegative signed" mode will + * accept signed values, but improperly encode and decode them if they are + * negative. In the future, the DEFAULT mode could be extended to support + * negative numbers in a backwards compatible way, and additional modes could be + * added to support different varint formats (e.g. zigzag encoding). + */ +enum class VarIntMode { DEFAULT, NONNEGATIVE_SIGNED }; + +template struct CheckVarIntMode { + constexpr CheckVarIntMode() { + static_assert(Mode != VarIntMode::DEFAULT || std::is_unsigned::value, + "Unsigned type required with mode DEFAULT."); + static_assert(Mode != VarIntMode::NONNEGATIVE_SIGNED || + std::is_signed::value, + "Signed type required with mode NONNEGATIVE_SIGNED."); + } +}; + +template +inline unsigned int GetSizeOfVarInt(I n) { + CheckVarIntMode(); int nRet = 0; while (true) { nRet++; @@ -363,7 +388,9 @@ template inline void WriteVarInt(CSizeComputer &os, I n); -template void WriteVarInt(Stream &os, I n) { +template +void WriteVarInt(Stream &os, I n) { + CheckVarIntMode(); uint8_t tmp[(sizeof(n) * 8 + 6) / 7]; int len = 0; while (true) { @@ -379,7 +406,9 @@ } while (len--); } -template I ReadVarInt(Stream &is) { +template +I ReadVarInt(Stream &is) { + CheckVarIntMode(); I n = 0; while (true) { uint8_t chData = ser_readdata8(is); @@ -398,7 +427,7 @@ } #define FLATDATA(obj) CFlatData((char *)&(obj), (char *)&(obj) + sizeof(obj)) -#define VARINT(obj) WrapVarInt(REF(obj)) +#define VARINT(obj, ...) WrapVarInt<__VA_ARGS__>(REF(obj)) #define COMPACTSIZE(obj) CCompactSize(REF(obj)) #define LIMITED_STRING(obj, n) LimitedString(REF(obj)) @@ -436,7 +465,7 @@ } }; -template class CVarInt { +template class CVarInt { protected: I &n; @@ -444,11 +473,11 @@ explicit CVarInt(I &nIn) : n(nIn) {} template void Serialize(Stream &s) const { - WriteVarInt(s, n); + WriteVarInt(s, n); } template void Unserialize(Stream &s) { - n = ReadVarInt(s); + n = ReadVarInt(s); } }; @@ -494,8 +523,9 @@ } }; -template CVarInt WrapVarInt(I &n) { - return CVarInt(n); +template +CVarInt WrapVarInt(I &n) { + return CVarInt{n}; } /** diff --git a/src/test/hash_tests.cpp b/src/test/hash_tests.cpp --- a/src/test/hash_tests.cpp +++ b/src/test/hash_tests.cpp @@ -171,13 +171,13 @@ uint32_t GetValue() { return value; } template void Serialize(Stream &s) const { - int nVersionDummy = 0; + unsigned int nVersionDummy = 0; ::Serialize(s, VARINT(nVersionDummy)); ::Serialize(s, VARINT(value)); } template void Unserialize(Stream &s) { - int nVersionDummy; + unsigned int nVersionDummy; ::Unserialize(s, VARINT(nVersionDummy)); ::Unserialize(s, VARINT(value)); } diff --git a/src/test/serialize_tests.cpp b/src/test/serialize_tests.cpp --- a/src/test/serialize_tests.cpp +++ b/src/test/serialize_tests.cpp @@ -186,8 +186,9 @@ CDataStream ss(SER_DISK, 0); CDataStream::size_type size = 0; for (int i = 0; i < 100000; i++) { - ss << VARINT(i); - size += ::GetSerializeSize(VARINT(i), 0, 0); + ss << VARINT(i, VarIntMode::NONNEGATIVE_SIGNED); + size += + ::GetSerializeSize(VARINT(i, VarIntMode::NONNEGATIVE_SIGNED), 0, 0); BOOST_CHECK(size == ss.size()); } @@ -200,7 +201,7 @@ // decode for (int i = 0; i < 100000; i++) { int j = -1; - ss >> VARINT(j); + ss >> VARINT(j, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_MESSAGE(i == j, "decoded:" << j << " expected:" << i); } @@ -213,37 +214,37 @@ BOOST_AUTO_TEST_CASE(varints_bitpatterns) { CDataStream ss(SER_DISK, 0); - ss << VARINT(0); + ss << VARINT(0, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_EQUAL(HexStr(ss), "00"); ss.clear(); - ss << VARINT(0x7f); + ss << VARINT(0x7f, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_EQUAL(HexStr(ss), "7f"); ss.clear(); - ss << VARINT((int8_t)0x7f); + ss << VARINT((int8_t)0x7f, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_EQUAL(HexStr(ss), "7f"); ss.clear(); - ss << VARINT(0x80); + ss << VARINT(0x80, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_EQUAL(HexStr(ss), "8000"); ss.clear(); ss << VARINT((uint8_t)0x80); BOOST_CHECK_EQUAL(HexStr(ss), "8000"); ss.clear(); - ss << VARINT(0x1234); + ss << VARINT(0x1234, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_EQUAL(HexStr(ss), "a334"); ss.clear(); - ss << VARINT((int16_t)0x1234); + ss << VARINT((int16_t)0x1234, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_EQUAL(HexStr(ss), "a334"); ss.clear(); - ss << VARINT(0xffff); + ss << VARINT(0xffff, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_EQUAL(HexStr(ss), "82fe7f"); ss.clear(); ss << VARINT((uint16_t)0xffff); BOOST_CHECK_EQUAL(HexStr(ss), "82fe7f"); ss.clear(); - ss << VARINT(0x123456); + ss << VARINT(0x123456, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_EQUAL(HexStr(ss), "c7e756"); ss.clear(); - ss << VARINT((int32_t)0x123456); + ss << VARINT((int32_t)0x123456, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_EQUAL(HexStr(ss), "c7e756"); ss.clear(); ss << VARINT(0x80123456U); @@ -255,7 +256,7 @@ ss << VARINT(0xffffffff); BOOST_CHECK_EQUAL(HexStr(ss), "8efefefe7f"); ss.clear(); - ss << VARINT(0x7fffffffffffffffLL); + ss << VARINT(0x7fffffffffffffffLL, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_EQUAL(HexStr(ss), "fefefefefefefefe7f"); ss.clear(); ss << VARINT(0xffffffffffffffffULL); diff --git a/src/txdb.cpp b/src/txdb.cpp --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -330,7 +330,7 @@ template void Unserialize(Stream &s) { uint32_t nCode = 0; // version - int nVersionDummy; + unsigned int nVersionDummy; ::Unserialize(s, VARINT(nVersionDummy)); // header code ::Unserialize(s, VARINT(nCode)); @@ -359,7 +359,7 @@ } } // coinbase height - ::Unserialize(s, VARINT(nHeight)); + ::Unserialize(s, VARINT(nHeight, VarIntMode::NONNEGATIVE_SIGNED)); } }; } // namespace diff --git a/src/undo.h b/src/undo.h --- a/src/undo.h +++ b/src/undo.h @@ -58,7 +58,7 @@ // Old versions stored the version number for the last spend of a // transaction's outputs. Non-final spends were indicated with // height = 0. - int nVersionDummy; + unsigned int nVersionDummy; ::Unserialize(s, VARINT(nVersionDummy)); }