bench: Construct CTxOut and COutPoint in a single expression #36019

1. 
   alexanderwiederin commented at 8:33 AM on August 19, 2026: contributor

   Replaces field-by-field mutation of CTxOut and COutPoint in two bench files with brace initialisation, which requires the size_t conversions to be made explicit.

   Noticed while looking at #35994, where switching the proposed fix-it to {} surfaces implicit narrowing conversions like these.

   The constructed values are unchanged.

   Note: Only the sites where a conversion is involved are included in this PR; the remaining field-by-field construction in bench/ would be covered by #35994's follow-ups.

2. DrahtBot added the label Tests on Aug 19, 2026
3. 
   DrahtBot commented at 8:33 AM on August 19, 2026: contributor

   <!--e57a25ab6845829454e8d69fc972939a-->

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   <!--006a51241073e994b41acfe9ec718e94-->

   Code Coverage & Benchmarks

   For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/36019.

   <!--021abf342d371248e50ceaed478a90ca-->

   Reviews

   See the guideline and AI policy for information on the review process.

   Type	Reviewers
   ACK	l0rinc, maflcko
   Stale ACK	josibake

   If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

   <!--174a7506f384e20aa4161008e828411d-->

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #35994 (Primitives: Combine assignments by purpleKarrot)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

   <!--5faf32d7da4f0f540f40219e4f7537a3-->

4. in src/bench/blockencodings.cpp:92 in 361ee92e3e

     88 | @@ -89,8 +89,7 @@ static void BlockEncodingBench(benchmark::Bench& bench, size_t n_pool, size_t n_
     89 |          tx.vin[0].scriptSig = CScript() << sigspam;
     90 |          tx.vin[0].scriptWitness.stack.push_back({1});
     91 |          tx.vout.resize(1);
     92 | -        tx.vout[0].scriptPubKey = CScript() << OP_1 << OP_EQUAL;
     93 | -        tx.vout[0].nValue = i;
     94 | +        tx.vout[0] = CTxOut{static_cast<CAmount>(i), CScript() << OP_1 << OP_EQUAL};
   

   ---

   ---

   

   maflcko commented at 8:37 AM on August 19, 2026:

           tx.vout = {CTxOut{static_cast<CAmount>(i), CScript() << OP_1 << OP_EQUAL}};
   

   maybe use an init-list?

   ---

   

   alexanderwiederin commented at 9:00 AM on August 19, 2026:

   Done - thanks!

5. alexanderwiederin force-pushed on Aug 19, 2026
6. DrahtBot added the label CI failed on Aug 19, 2026
7. DrahtBot removed the label CI failed on Aug 19, 2026
8. in src/bench/mempool_ephemeral_spends.cpp:54 in 5d53932496

     49 | @@ -50,9 +50,8 @@ static void MempoolCheckEphemeralSpends(benchmark::Bench& bench)
     50 |      tx1.vin.resize(1);
     51 |      tx1.vout.resize(number_outputs);
     52 |      for (size_t i = 0; i < tx1.vout.size(); i++) {
     53 | -        tx1.vout[i].scriptPubKey = CScript();
     54 |          // Each output progressively larger
     55 | -        tx1.vout[i].nValue = i * CENT;
     56 | +        tx1.vout[i] = CTxOut{static_cast<CAmount>(i) * CENT, CScript()};
   

   ---

   ---

   

   josibake commented at 2:41 PM on August 19, 2026:

   I think slightly better is:

   tx1.vout.reserve(number_outputs);
   
   for (size_t i = 0; i < number_outputs; ++i) {
       tx1.vout.push_back(
           CTxOut{static_cast<CAmount>(i) * CENT, CScript{}}
       );
   }
   

   .. as I find it improves the readability and also keeps the type being constructed clearly visible. If performance were a concern you could do:

   tx1.vout.reserve(number_outputs);
   
   for (size_t i = 0; i < number_outputs; ++i) {
       tx1.vout.emplace_back(
           static_cast<CAmount>(i) * CENT, 
           CScript{}
       );
   }
   

   but considering this really hurts the readability I wouldn't suggest it here.

   ---

   

   alexanderwiederin commented at 3:53 PM on August 19, 2026:

   Thanks! push_back trips modernize-use-emplace, so the options are emplace_back or keeping the indexed assignment.

   ---

   

   josibake commented at 4:01 PM on August 19, 2026:

   Boo! I'd say indexed assignment, then, for readability. I think reserve might still be preferable tho? Not sure tho if it makes any difference with index assignment.

   ---

   

   alexanderwiederin commented at 4:13 PM on August 19, 2026:

   reserve only sets capacity, so indexed assignment still needs resize.

   ---

   

   l0rinc commented at 5:36 PM on August 20, 2026:

   nit: These static_cast expressions have a horrible signal-to-noise ratio.

   If you need to touch these lines again, please consider functional cast notation, similar to the explicit CAmount construction proposed in #35511.

   Also applies to blockencodings.cpp.

           tx1.vout[i] = {CAmount(i) * CENT, CScript{}};
   

   ---

   

   alexanderwiederin commented at 11:25 PM on August 20, 2026:

   Applied - thanks! Also in blockencodings.cpp.

   Kept CTxOut named rather than eliding it for readability; see #36019 (review)

9. in src/bench/mempool_ephemeral_spends.cpp:63 in 5d53932496

     59 | @@ -61,8 +60,7 @@ static void MempoolCheckEphemeralSpends(benchmark::Bench& bench)
     60 |      CMutableTransaction tx2;
     61 |      tx2.vin.resize(tx1.vout.size());
     62 |      for (size_t i = 0; i < tx2.vin.size(); i++) {
     63 | -        tx2.vin[i].prevout.hash = parent_txid;
     64 | -        tx2.vin[i].prevout.n = i;
     65 | +        tx2.vin[i].prevout = COutPoint{parent_txid, static_cast<uint32_t>(i)};
   

   ---

   ---

   

   josibake commented at 2:50 PM on August 19, 2026:

   Same suggestion as above with:

   tx2.reserve(tx1.vout.size());
   for (size_t i = 0; i < tx2.vin.size(); i++) {
       tx2.push_back(
           COutPoint{parent_txid, static_cast<uint32_t>(i)}
       );
   }
   

   ---

   

   alexanderwiederin commented at 4:24 PM on August 19, 2026:

   Did you mean tx2.vin? It holds CTxIn rather than COutPoint. It would have be:

   tx2.vin.reserve(tx1.vout.size());
   for (size_t i = 0; i < tx1.vout.size(); i++) {
       tx2.vin.emplace_back(COutPoint{parent_txid, static_cast<uint32_t>(i)});
   }
   

   which constructs the whole input rather than just setting prevout (same values; CTxIn's defaults match what resize gave).

   What do you think?

   Edit: note the loop bound also has to change from tx2.vin.size() to tx2.vout.size()

   ---

   

   josibake commented at 5:28 PM on August 19, 2026:

   Yeah, sorry, was copying and pasting and typing fast. Considering push back is no good, I think leaving this is fine.

10. josibake approved
11. 
    josibake commented at 2:52 PM on August 19, 2026: member

    ACK https://github.com/bitcoin/bitcoin/commit/5d539324962785b811af2849beb639c667bc6d42

    LGTM! Left a non-blocking readability suggestion.

12. 
    l0rinc commented at 6:38 PM on August 19, 2026: contributor

    ~Partially fixed by #35859, thanks @benthecarman~ wrong thread

13. 
    maflcko commented at 7:20 AM on August 20, 2026: member

    Note: Only the sites where a conversion is involved are included in this PR; the remaining field-by-field construction in bench/ would be covered by #35994's follow-ups.

    Not sure about this. src/bench/duplicate_inputs.cpp etc should also be init-list (https://github.com/bitcoin/bitcoin/pull/36019#discussion_r3811388178)?

14. 
    alexanderwiederin commented at 10:35 AM on August 20, 2026: contributor

    Note: Only the sites where a conversion is involved are included in this PR; the remaining field-by-field construction in bench/ would be covered by #35994's follow-ups.

    Not sure about this. src/bench/duplicate_inputs.cpp etc should also be init-list (#36019 (comment))?

    Happy to extend. Do you want every field-by-field construction in src/bench/ converted, i.e. init-list where the vector is fixed-size and direct assignment where it's filled in a loop? That would be duplicate_inputs.cpp, mempool_eviction.cpp, wallet_create_tx.cpp, rpc_mempool.cpp and mempool_stress.cpp.

15. in src/bench/mempool_ephemeral_spends.cpp:64 in 5d53932496

      59 | @@ -61,8 +60,7 @@ static void MempoolCheckEphemeralSpends(benchmark::Bench& bench)
      60 |      CMutableTransaction tx2;
      61 |      tx2.vin.resize(tx1.vout.size());
      62 |      for (size_t i = 0; i < tx2.vin.size(); i++) {
      63 | -        tx2.vin[i].prevout.hash = parent_txid;
      64 | -        tx2.vin[i].prevout.n = i;
      65 | +        tx2.vin[i].prevout = COutPoint{parent_txid, static_cast<uint32_t>(i)};
      66 |      }
    

    ---

    ---

    

    l0rinc commented at 5:38 PM on August 20, 2026:

    Since the input count ultimately comes from int number_outputs, could we use the outpoint index type directly and avoid the cast?

        for (uint32_t i{0}; i < tx2.vin.size(); ++i) {
            tx2.vin[i].prevout = {parent_txid, i};
        }
    

    ---

    

    alexanderwiederin commented at 11:26 PM on August 20, 2026:

    Thanks! Kept COutPoint{...} for the same reason as above.

    ---

    

    l0rinc commented at 11:29 PM on August 20, 2026:

    not sure I get the reason, can prevout or vout have any other type?

16. bench: Construct CTxOut and COutPoint in a single expression

    Replace separate member assignments with construction, using brace
    initialization so the size_t to CAmount conversions have to be explicit.
    Use uint32_t for the loop index feeding COutPoint::n, which avoids
    conversion entirely.

    950bdb763e
17. alexanderwiederin force-pushed on Aug 20, 2026
18. 
    l0rinc commented at 12:14 AM on August 21, 2026: contributor

    code review ACK 950bdb763e17bf7dc527b8aecb0d392062aec437

19. DrahtBot requested review from josibake on Aug 21, 2026
20. 
    maflcko commented at 5:19 AM on August 21, 2026: member

    review ACK 950bdb763e17bf7dc527b8aecb0d392062aec437 🐯

    <details><summary>Show signature</summary>

    Signature:

    untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
    RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
    trusted comment: review ACK 950bdb763e17bf7dc527b8aecb0d392062aec437 🐯
    AESkApduw+PXC+Ac099b6fVVLq6g29lgLYwU6gFp7DTs0WyuRolPsfZR4gRhAc+yEntwOUnirdE91CmV9504Dw==
    

    </details>

21. fanquake merged this on Aug 21, 2026
22. fanquake closed this on Aug 21, 2026

    ---

23. 
    maflcko commented at 7:39 AM on August 21, 2026: member

    Note: Only the sites where a conversion is involved are included in this PR; the remaining field-by-field construction in bench/ would be covered by #35994's follow-ups.

    Not sure about this. src/bench/duplicate_inputs.cpp etc should also be init-list (#36019 (comment))?

    Happy to extend. Do you want every field-by-field construction in src/bench/ converted, i.e. init-list where the vector is fixed-size and direct assignment where it's filled in a loop?

    Yeah, I mostly think that if we want to use cleaner init-list or cleaner direct-assign for CTxOut and COutpont, then it should be done in larger batches in the tests. Creating a separate 4-line-diff pull for every single test file doesn't really scale.

24. 
    josibake commented at 8:01 AM on August 21, 2026: member

    post merge reACK https://github.com/bitcoin/bitcoin/commit/950bdb763e17bf7dc527b8aecb0d392062aec437

Contributors

alexanderwiederinDrahtBotmaflckojosibakel0rinc

Review Requested
josibake