redeclare nChainTx to use uint64_t #29331

1. 
   russeree commented at 9:44 pm on January 26, 2024: contributor

   Following up on #29258 the type of nChainTx has now been changed to a uint64_t allowing for the counting of TXs well into the future. The note about changing the type in 2024 was also removed. This passes all extended tests.
2. 
   DrahtBot commented at 9:44 pm on January 26, 2024: contributor

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

   Code Coverage

   For detailed information about the code coverage, see the test coverage report.

   Reviews

   See the guideline for information on the review process. A summary of reviews will appear here.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #29370 (assumeutxo: Get rid of faked nTx and nChainTx values by ryanofsky)
   • #28608 (assumeutxo state and locking cleanup by ryanofsky)

   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.

3. russeree force-pushed on Jan 26, 2024
4. russeree force-pushed on Jan 26, 2024
5. russeree force-pushed on Jan 26, 2024
6. russeree force-pushed on Jan 26, 2024
7. russeree force-pushed on Jan 26, 2024
8. russeree marked this as a draft on Jan 26, 2024
9. DrahtBot added the label CI failed on Jan 26, 2024
10. 
    DrahtBot commented at 11:49 pm on January 26, 2024: contributor

    🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the documentation.

    Possibly this is due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    Leave a comment here, if you need help tracking down a confusing failure.

    Debug: https://github.com/bitcoin/bitcoin/runs/20917625235

11. DrahtBot removed the label CI failed on Jan 27, 2024
12. DrahtBot added the label CI failed on Jan 27, 2024
13. redeclare nChainTx to use uint64_t ecabf53cd2
14. russeree force-pushed on Jan 27, 2024
15. russeree marked this as ready for review on Jan 27, 2024
16. DrahtBot removed the label CI failed on Jan 27, 2024
17. 
    dergoegge commented at 11:21 am on January 29, 2024: member

    Would you mind adding a test? It should be possible to fake a large nChainTx in a test using assume utxo
18. 
    mzumsande commented at 6:21 pm on January 29, 2024: contributor

    It should be possible to fake a large nChainTx in a test using assume utxo

    It’s annoying that each test doing something like that would need to add an entry to the regtest chainparams, maybe my suggestion here to allow that per RPC could make sense (not necessarily in this PR).

19. 
    dergoegge commented at 10:53 am on January 30, 2024: member

    It’s annoying that each test doing something like that would need to add an entry to the regtest chainparams, maybe my suggestion #29261 (comment) to allow that per RPC could make sense (not necessarily in this PR).

    I was thinking of a unit test, so no need for an RPC but yes some way to add assumeutxo data for tests would be needed (I assumed we already have this).

20. 
    maflcko commented at 12:57 pm on February 5, 2024: member

    The note about changing the type in 2024 was also removed. This passes all extended tests.

    I don’t think you need to detail what is changed and if the tests pass in the description. It would be better to focus on information that is useful to reviewers. For example, a measurement of memory before and after.

    Also, when changing the type, you’ll have to check all places where this is used, and update them, if needed. For example, casts should be removed and other types need to be increased in width to hold the new values.

21. 
    maflcko commented at 10:12 am on February 12, 2024: member

    Are you still working on this? If not, I am happy to pick up.
22. 
    russeree commented at 1:39 pm on February 12, 2024: contributor

    Could I please have just a bit more time to work on this?

    Thanks, Reese

23. 
    fanquake commented at 1:41 pm on February 27, 2024: member

    @russeree are you still following up here?
24. 
    russeree commented at 9:20 pm on February 28, 2024: contributor

    @russeree are you still following up here?

    I would be willing to hand this off now, primary issue is the inability to allocate the proper amount of time due to other work commitments.

    Sorry if this caused any substantial setbacks.

25. 
    fanquake commented at 9:32 pm on February 28, 2024: member

    No worries. @maflcko may pick it up.
26. fanquake closed this on Feb 28, 2024

    ---

27. fanquake added the label Up for grabs on Feb 28, 2024
28. maflcko removed the label Up for grabs on Mar 15, 2024

Contributors
russeree DrahtBot dergoegge mzumsande maflcko fanquake