Skip to content

Commit 33adc75

Browse files
achow101achow101
committed
Merge bitcoin/bitcoin#30765: refactor: Allow CScript's operator<< to accept spans, not just vectors
5e190cd Replace CScript _hex_v_u8 appends with _hex (Lőrinc) cac846c Allow CScript's operator<< to accept spans, not just vectors (Lőrinc) c78d8ff prevector: avoid GCC bogus warnings in insert method (Lőrinc) Pull request description: Split out of bitcoin/bitcoin#30377 (comment). Replace `_hex_v_u8` for `CScript` appends to `_hex`, to skip vector conversion before serializing to the `prevector` in `CScript`. To enable both `unsigned char` and `std::byte` values, I've extracted the existing serialization to append the size & data in separate private methods to clarify that it does more than just a simple data insertion. There were also discussion on eliminating the operators here completely to obviate when we're serializing fixed-size collections as raw bytes, and when we're prefixing them with their size - should also be done in a separate PR. ACKs for top commit: achow101: ACK 5e190cd ryanofsky: Code review ACK 5e190cd. Looks good! hodlinator: re-ACK 5e190cd Tree-SHA512: 27a646629e017b2a05416d5eb964dda8b25b900d466221eff7bfa1339ded443e1c5c4cf8ff20cb3bba915a2603787a9fa6f6ec12bc0b9415d9eb07b57289192b
2 parents 0894748 + 5e190cd commit 33adc75

File tree

7 files changed

+64
-43
lines changed

7 files changed

+64
-43
lines changed

‎src/kernel/chainparams.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ static CBlock CreateGenesisBlock(const char* pszTimestamp, const CScript& genesi
7373
static CBlock CreateGenesisBlock(uint32_t nTime, uint32_t nNonce, uint32_t nBits, int32_t nVersion, const CAmount& genesisReward)
7474
{
7575
const char* pszTimestamp = "The Times 03/Jan/2009 Chancellor on brink of second bailout for banks";
76-
const CScript genesisOutputScript = CScript() << "04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5f"_hex_v_u8 << OP_CHECKSIG;
76+
const CScript genesisOutputScript = CScript() << "04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5f"_hex << OP_CHECKSIG;
7777
return CreateGenesisBlock(pszTimestamp, genesisOutputScript, nTime, nNonce, nBits, nVersion, genesisReward);
7878
}
7979

@@ -353,7 +353,7 @@ class CTestNet4Params : public CChainParams {
353353
m_assumed_chain_state_size = 0;
354354

355355
const char* testnet4_genesis_msg = "03/May/2024 000000000000000000001ebd58c244970b3aa9d783bb001011fbe8ea8e98e00e";
356-
const CScript testnet4_genesis_script = CScript() << "000000000000000000000000000000000000000000000000000000000000000000"_hex_v_u8 << OP_CHECKSIG;
356+
const CScript testnet4_genesis_script = CScript() << "000000000000000000000000000000000000000000000000000000000000000000"_hex << OP_CHECKSIG;
357357
genesis = CreateGenesisBlock(testnet4_genesis_msg,
358358
testnet4_genesis_script,
359359
1714777860,

‎src/prevector.h

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,8 @@ class prevector {
363363
change_capacity(new_size + (new_size >> 1));
364364
}
365365
T* ptr = item_ptr(p);
366-
memmove(ptr + 1, ptr, (size() - p) * sizeof(T));
366+
T* dst = ptr + 1;
367+
memmove(dst, ptr, (size() - p) * sizeof(T));
367368
_size++;
368369
new(static_cast<void*>(ptr)) T(value);
369370
return iterator(ptr);
@@ -376,7 +377,8 @@ class prevector {
376377
change_capacity(new_size + (new_size >> 1));
377378
}
378379
T* ptr = item_ptr(p);
379-
memmove(ptr + count, ptr, (size() - p) * sizeof(T));
380+
T* dst = ptr + count;
381+
memmove(dst, ptr, (size() - p) * sizeof(T));
380382
_size += count;
381383
fill(item_ptr(p), count, value);
382384
}
@@ -390,7 +392,8 @@ class prevector {
390392
change_capacity(new_size + (new_size >> 1));
391393
}
392394
T* ptr = item_ptr(p);
393-
memmove(ptr + count, ptr, (size() - p) * sizeof(T));
395+
T* dst = ptr + count;
396+
memmove(dst, ptr, (size() - p) * sizeof(T));
394397
_size += count;
395398
fill(ptr, first, last);
396399
}

‎src/script/script.h

Lines changed: 36 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include <cstdint>
1818
#include <cstring>
1919
#include <limits>
20+
#include <span>
2021
#include <stdexcept>
2122
#include <string>
2223
#include <type_traits>
@@ -412,6 +413,32 @@ bool GetScriptOp(CScriptBase::const_iterator& pc, CScriptBase::const_iterator en
412413
/** Serialized script, used inside transaction inputs and outputs */
413414
class CScript : public CScriptBase
414415
{
416+
private:
417+
inline void AppendDataSize(const uint32_t size)
418+
{
419+
if (size < OP_PUSHDATA1) {
420+
insert(end(), static_cast<value_type>(size));
421+
} else if (size <= 0xff) {
422+
insert(end(), OP_PUSHDATA1);
423+
insert(end(), static_cast<value_type>(size));
424+
} else if (size <= 0xffff) {
425+
insert(end(), OP_PUSHDATA2);
426+
value_type data[2];
427+
WriteLE16(data, size);
428+
insert(end(), std::cbegin(data), std::cend(data));
429+
} else {
430+
insert(end(), OP_PUSHDATA4);
431+
value_type data[4];
432+
WriteLE32(data, size);
433+
insert(end(), std::cbegin(data), std::cend(data));
434+
}
435+
}
436+
437+
void AppendData(std::span<const value_type> data)
438+
{
439+
insert(end(), data.begin(), data.end());
440+
}
441+
415442
protected:
416443
CScript& push_int64(int64_t n)
417444
{
@@ -463,35 +490,19 @@ class CScript : public CScriptBase
463490
return *this;
464491
}
465492

466-
CScript& operator<<(const std::vector<unsigned char>& b) LIFETIMEBOUND
493+
CScript& operator<<(std::span<const std::byte> b) LIFETIMEBOUND
467494
{
468-
if (b.size() < OP_PUSHDATA1)
469-
{
470-
insert(end(), (unsigned char)b.size());
471-
}
472-
else if (b.size() <= 0xff)
473-
{
474-
insert(end(), OP_PUSHDATA1);
475-
insert(end(), (unsigned char)b.size());
476-
}
477-
else if (b.size() <= 0xffff)
478-
{
479-
insert(end(), OP_PUSHDATA2);
480-
uint8_t _data[2];
481-
WriteLE16(_data, b.size());
482-
insert(end(), _data, _data + sizeof(_data));
483-
}
484-
else
485-
{
486-
insert(end(), OP_PUSHDATA4);
487-
uint8_t _data[4];
488-
WriteLE32(_data, b.size());
489-
insert(end(), _data, _data + sizeof(_data));
490-
}
491-
insert(end(), b.begin(), b.end());
495+
AppendDataSize(b.size());
496+
AppendData({reinterpret_cast<const value_type*>(b.data()), b.size()});
492497
return *this;
493498
}
494499

500+
// For compatibility reasons. In new code, prefer using std::byte instead of uint8_t.
501+
CScript& operator<<(std::span<const value_type> b) LIFETIMEBOUND
502+
{
503+
return *this << std::as_bytes(b);
504+
}
505+
495506
bool GetOp(const_iterator& pc, opcodetype& opcodeRet, std::vector<unsigned char>& vchRet) const
496507
{
497508
return GetScriptOp(pc, end(), opcodeRet, &vchRet);

‎src/test/miner_tests.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -608,7 +608,7 @@ void MinerTestingSetup::TestPrioritisedMining(const CScript& scriptPubKey, const
608608
BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
609609
{
610610
// Note that by default, these tests run with size accounting enabled.
611-
CScript scriptPubKey = CScript() << "04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5f"_hex_v_u8 << OP_CHECKSIG;
611+
CScript scriptPubKey = CScript() << "04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5f"_hex << OP_CHECKSIG;
612612
std::unique_ptr<CBlockTemplate> pblocktemplate;
613613

614614
CTxMemPool& tx_mempool{*m_node.mempool};

‎src/test/script_tests.cpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1368,6 +1368,13 @@ static CScript ScriptFromHex(const std::string& str)
13681368
return ToScript(*Assert(TryParseHex(str)));
13691369
}
13701370

1371+
BOOST_AUTO_TEST_CASE(script_byte_array_u8_vector_equivalence)
1372+
{
1373+
const CScript scriptPubKey1 = CScript() << "04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5f"_hex_v_u8 << OP_CHECKSIG;
1374+
const CScript scriptPubKey2 = CScript() << "04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5f"_hex << OP_CHECKSIG;
1375+
BOOST_CHECK(scriptPubKey1 == scriptPubKey2);
1376+
}
1377+
13711378
BOOST_AUTO_TEST_CASE(script_FindAndDelete)
13721379
{
13731380
// Exercise the FindAndDelete functionality
@@ -1421,7 +1428,7 @@ BOOST_AUTO_TEST_CASE(script_FindAndDelete)
14211428
// prefix, leaving 02ff03 which is push-two-bytes:
14221429
s = ToScript("0302ff030302ff03"_hex);
14231430
d = ToScript("03"_hex);
1424-
expect = CScript() << "ff03"_hex_v_u8 << "ff03"_hex_v_u8;
1431+
expect = CScript() << "ff03"_hex << "ff03"_hex;
14251432
BOOST_CHECK_EQUAL(FindAndDelete(s, d), 2);
14261433
BOOST_CHECK(s == expect);
14271434

‎src/test/transaction_tests.cpp

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -852,24 +852,24 @@ BOOST_AUTO_TEST_CASE(test_IsStandard)
852852
CheckIsNotStandard(t, "scriptpubkey");
853853

854854
// MAX_OP_RETURN_RELAY-byte TxoutType::NULL_DATA (standard)
855-
t.vout[0].scriptPubKey = CScript() << OP_RETURN << "04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef3804678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38"_hex_v_u8;
855+
t.vout[0].scriptPubKey = CScript() << OP_RETURN << "04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef3804678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38"_hex;
856856
BOOST_CHECK_EQUAL(MAX_OP_RETURN_RELAY, t.vout[0].scriptPubKey.size());
857857
CheckIsStandard(t);
858858

859859
// MAX_OP_RETURN_RELAY+1-byte TxoutType::NULL_DATA (non-standard)
860-
t.vout[0].scriptPubKey = CScript() << OP_RETURN << "04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef3804678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef3800"_hex_v_u8;
860+
t.vout[0].scriptPubKey = CScript() << OP_RETURN << "04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef3804678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef3800"_hex;
861861
BOOST_CHECK_EQUAL(MAX_OP_RETURN_RELAY + 1, t.vout[0].scriptPubKey.size());
862862
CheckIsNotStandard(t, "scriptpubkey");
863863

864864
// Data payload can be encoded in any way...
865-
t.vout[0].scriptPubKey = CScript() << OP_RETURN << ""_hex_v_u8;
865+
t.vout[0].scriptPubKey = CScript() << OP_RETURN << ""_hex;
866866
CheckIsStandard(t);
867-
t.vout[0].scriptPubKey = CScript() << OP_RETURN << "00"_hex_v_u8 << "01"_hex_v_u8;
867+
t.vout[0].scriptPubKey = CScript() << OP_RETURN << "00"_hex << "01"_hex;
868868
CheckIsStandard(t);
869869
// OP_RESERVED *is* considered to be a PUSHDATA type opcode by IsPushOnly()!
870-
t.vout[0].scriptPubKey = CScript() << OP_RETURN << OP_RESERVED << -1 << 0 << "01"_hex_v_u8 << 2 << 3 << 4 << 5 << 6 << 7 << 8 << 9 << 10 << 11 << 12 << 13 << 14 << 15 << 16;
870+
t.vout[0].scriptPubKey = CScript() << OP_RETURN << OP_RESERVED << -1 << 0 << "01"_hex << 2 << 3 << 4 << 5 << 6 << 7 << 8 << 9 << 10 << 11 << 12 << 13 << 14 << 15 << 16;
871871
CheckIsStandard(t);
872-
t.vout[0].scriptPubKey = CScript() << OP_RETURN << 0 << "01"_hex_v_u8 << 2 << "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"_hex_v_u8;
872+
t.vout[0].scriptPubKey = CScript() << OP_RETURN << 0 << "01"_hex << 2 << "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"_hex;
873873
CheckIsStandard(t);
874874

875875
// ...so long as it only contains PUSHDATA's
@@ -883,13 +883,13 @@ BOOST_AUTO_TEST_CASE(test_IsStandard)
883883

884884
// Only one TxoutType::NULL_DATA permitted in all cases
885885
t.vout.resize(2);
886-
t.vout[0].scriptPubKey = CScript() << OP_RETURN << "04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38"_hex_v_u8;
886+
t.vout[0].scriptPubKey = CScript() << OP_RETURN << "04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38"_hex;
887887
t.vout[0].nValue = 0;
888-
t.vout[1].scriptPubKey = CScript() << OP_RETURN << "04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38"_hex_v_u8;
888+
t.vout[1].scriptPubKey = CScript() << OP_RETURN << "04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38"_hex;
889889
t.vout[1].nValue = 0;
890890
CheckIsNotStandard(t, "multi-op-return");
891891

892-
t.vout[0].scriptPubKey = CScript() << OP_RETURN << "04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38"_hex_v_u8;
892+
t.vout[0].scriptPubKey = CScript() << OP_RETURN << "04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38"_hex;
893893
t.vout[1].scriptPubKey = CScript() << OP_RETURN;
894894
CheckIsNotStandard(t, "multi-op-return");
895895

‎src/wallet/test/ismine_tests.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -684,7 +684,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
684684
BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0]));
685685

686686
scriptPubKey.clear();
687-
scriptPubKey << OP_0 << "aabb"_hex_v_u8;
687+
scriptPubKey << OP_0 << "aabb"_hex;
688688

689689
result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey);
690690
BOOST_CHECK_EQUAL(result, ISMINE_NO);
@@ -699,7 +699,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
699699
BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0]));
700700

701701
scriptPubKey.clear();
702-
scriptPubKey << OP_16 << "aabb"_hex_v_u8;
702+
scriptPubKey << OP_16 << "aabb"_hex;
703703

704704
result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey);
705705
BOOST_CHECK_EQUAL(result, ISMINE_NO);

0 commit comments

Comments
 (0)