Differential D8203

[backport#16400] refactor: Rewrite AcceptToMemoryPoolWorker() using smaller parts
ClosedPublic
Actions

Authored by majcosta on Oct 30 2020, 22:48.

Details

Reviewers

deadalnix

Group Reviewers

Restricted Project

Commits

rABC3b83c27d9a15: [backport#16400] refactor: Rewrite AcceptToMemoryPoolWorker() using smaller…

Summary

4a87c5cfdf7dd72d999ebeaf17db6695a7c6298d [refactor] Rewrite AcceptToMemoryPoolWorker() using smaller parts (Suhas Daftuar)

Pull request description:

This is in preparation for re-using these validation components for a new version of AcceptToMemoryPool() that can operate on multiple transactions ("package relay").

Because of how our code handles SigChecks I could not remove the code commented under "Validate input
scripts against standard script flags" from the new PreChecks function into the PR's new PolicyScriptChecks
because since D5128 anything related to that number can only be run after CheckInputs.

The other code in the above new function was related to SegWit therefore I just removed the
PolicyScriptChecks function altogether.

Other missing members of the Workspace and ATMPArgs that were unused due to us not having RBF
were also taken out.

Backport of Core PR16400

Test Plan

ninja all check check-functional

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

majcosta created this revision.Oct 30 2020, 22:48

Herald added a reviewer: Restricted Project. · View Herald TranscriptOct 30 2020, 22:48

majcosta requested review of this revision.Oct 30 2020, 22:48

teamcity edited the summary of this revision. (Show Details)Oct 30 2020, 22:49

majcosta edited the summary of this revision. (Show Details)Oct 30 2020, 22:55

Harbormaster completed remote builds in B13780: Diff 25295.Oct 30 2020, 23:01

deadalnix requested changes to this revision.Oct 31 2020, 15:35

deadalnix added inline comments.

src/validation.cpp
444 ↗	(On Diff #25295)	I don't see what is gained except confusion here, creating many aliases to the same thing. Also, hello temporal dependency. This code is going to be broken at some point, this is 100% guaranteed, it is too fragile.
445 ↗	(On Diff #25295)	These variable definition at 25km from where they are being use makes the code very unreadable.
448 ↗	(On Diff #25295)	Considering this is the only use of m_config, this departure from the source material seems completely unnecessary.
647 ↗	(On Diff #25295)	That seems superfluous.
713 ↗	(On Diff #25295)	reflow