refactor: remove duplicate code from BlockAssembler #24364

1. 
   jamesob commented at 2:19 am on February 17, 2022: member

   Found while reminding myself how transactions are chosen for blocks. Take it or leave it!
2. refactor: remove duplicate code from BlockAssembler 0f40d65321
3. DrahtBot added the label Refactoring on Feb 17, 2022
4. 
   laanwj commented at 4:22 pm on February 23, 2022: member

   Concept ACK
5. theStack approved
6. 
   theStack commented at 10:19 am on March 1, 2022: contributor

   Concept and code-review ACK 0f40d653218789aa176ca2f844e3222d2ad890a3
7. 
   glozow commented at 1:21 pm on March 1, 2022: member

   code review ACK 0f40d653218789aa176ca2f844e3222d2ad890a3
8. fanquake added this to the milestone 24.0 on Mar 2, 2022
9. 
   maflcko commented at 4:18 pm on March 8, 2022: member

   Can anyone explain why this is correct? Why should GetModifiedFee be equal to GetFee??
10. 
    jamesob commented at 4:24 pm on March 8, 2022: member

    Can anyone explain why this is correct? Why should GetModifiedFee be equal to GetFee??

    Good point. This is not correct. :)

    Probably a comment and/or test should be added.

11. jamesob closed this on Mar 8, 2022

    ---

12. 
    glozow commented at 4:26 pm on March 8, 2022: member

    Ah that’s true, but it’s very weird that they’re not the same. Why does update_for_parent_inclusion use GetFee() and not GetModifiedFee() to update nModFeesWithAncestors?
13. 
    maflcko commented at 4:34 pm on March 8, 2022: member

    modified fee is already applied recursively, so if you apply it repeatedly it would accumulate?

    Edit: This is nonsense

14. 
    glozow commented at 4:40 pm on March 8, 2022: member

    What do you mean by recursively? Isn’t nModFeesWithAncestors the sum of ours + all ancestors’ modified fees? So if you’re updating for parent inclusion, using parent.GetFee() instead of parent.GetModifiedFee() means you’re still including the prioritisation from your parent, which would be incorrect.
15. 
    theStack commented at 5:29 pm on March 8, 2022: contributor

    Can anyone explain why this is correct? Why should GetModifiedFee be equal to GetFee??

    Oh snap, I obviously totally overlooked this in my review :/

16. 
    maflcko commented at 9:07 am on May 6, 2022: member

    This can be reopened, now that #https://github.com/bitcoin/bitcoin/pull/24538 is merged.
17. maflcko referenced this in commit b2e7811c62 on May 6, 2022
18. sidhujag referenced this in commit c8d2687323 on May 9, 2022
19. 
    fanquake commented at 2:45 pm on September 13, 2022: member

    @jamesob @glozow want to reopen / pick this up?
20. fanquake removed this from the milestone 24.0 on Sep 13, 2022
21. 
    glozow commented at 3:10 pm on September 13, 2022: member

    Concept ACK to reopen. Agree not necessary for v24.0.
22. jamesob reopened this on Sep 13, 2022

    ---

23. 
    DrahtBot commented at 1:01 pm on September 23, 2022: contributor

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

    Conflicts

    No conflicts as of last run.

24. 
    glozow commented at 9:57 am on October 6, 2022: member

    ACK 0f40d653218789aa176ca2f844e3222d2ad890a3
25. glozow merged this on Oct 6, 2022
26. glozow closed this on Oct 6, 2022

    ---

27. sidhujag referenced this in commit 22d943f5ee on Oct 6, 2022
28. Fabcien referenced this in commit ce0b1b9a24 on Jan 6, 2023
29. bitcoin locked this on Oct 6, 2023

Contributors
jamesob laanwj theStack glozow maflcko fanquake DrahtBot

Labels
Refactoring