refactor: CheckFinalTx pass by reference instead of pointer #22282

1. 
   klementtan commented at 9:13 am on June 19, 2021: contributor

   Rationale This PR fixes TODO from #21055

   https://github.com/bitcoin/bitcoin/blob/965e93743454112c0c3c66bf24852f63ee07b862/src/validation.cpp#L183

   #20750 (review) for additional information.

   Changes to CheckFinalTx: active_chain_tip to be passed by reference instead of pointer and shift & of tx to be left-attaching

2. refactor: CheckFinalTx pass by reference instead of pointer 10caa3e4b1
3. DrahtBot added the label Mempool on Jun 19, 2021
4. DrahtBot added the label Refactoring on Jun 19, 2021
5. DrahtBot added the label Validation on Jun 19, 2021
6. klementtan renamed this:
   refactor: CheckFinalTx pass by reference instead of ptr
   refactor: CheckFinalTx pass by reference instead of pointer
   on Jun 19, 2021
7. MarcoFalke removed the label Mempool on Jun 19, 2021
8. MarcoFalke removed the label Validation on Jun 19, 2021
9. theStack approved
10. 
    theStack commented at 1:04 pm on June 19, 2021: member

    Code-review ACK 10caa3e4b167dae26d52838ad19f8c137c9c0a14 🥥
11. 
    benthecarman commented at 6:41 am on June 20, 2021: contributor

    ack 10caa3e4b167dae26d52838ad19f8c137c9c0a14
12. 
    practicalswift commented at 11:24 am on June 20, 2021: contributor

    Concept ACK

    Candidates for similar treatment can be found using the git grep commands in #19062 :)

13. 
    klementtan commented at 1:03 pm on June 20, 2021: contributor

    Candidates for similar treatment can be found using the git grep commands in #19062 :)

    Thanks for informing me be about this. Added a comment on the issue to reference this PR. However, I think I will keep the scope of this PR to CheckFianlTx to make it easy to review.

14. 
    DrahtBot commented at 9:49 pm on August 10, 2021: member

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

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #22677 (cut the validation <-> txmempool circular dependency 2/2 by glozow)

    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.

15. in src/validation.cpp:183 in 10caa3e4b1

    176@@ -177,10 +177,9 @@ bool CheckInputScripts(const CTransaction& tx, TxValidationState& state,
    177                        std::vector<CScriptCheck>* pvChecks = nullptr)
    178                        EXCLUSIVE_LOCKS_REQUIRED(cs_main);
    179 
    180-bool CheckFinalTx(const CBlockIndex* active_chain_tip, const CTransaction &tx, int flags)
    181+bool CheckFinalTx(const CBlockIndex& active_chain_tip, const CTransaction& tx, int flags)
    182 {
    183     AssertLockHeld(cs_main);
    184-    assert(active_chain_tip); // TODO: Make active_chain_tip a reference
    

    ---

    ---

    

    MarcoFalke commented at 5:45 pm on December 1, 2021:

    This removes an assertion and turns a (theoretical) crash into UB
16. 
    DrahtBot commented at 6:39 pm on December 1, 2021: member

    🐙 This pull request conflicts with the target branch and needs rebase.

    Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a “draft”.

17. DrahtBot added the label Needs rebase on Dec 1, 2021
18. 
    klementtan commented at 5:47 pm on December 12, 2021: contributor

    Closing this PR as this refactor will remove an assertion.
19. klementtan closed this on Dec 12, 2021

    ---

20. 
    MarcoFalke commented at 8:01 am on December 13, 2021: member

    As long as you preserve the assertion, it should be ok.
21. DrahtBot locked this on Dec 13, 2022

Contributors
klementtan theStack benthecarman practicalswift DrahtBot MarcoFalke

Labels
Refactoring Needs rebase