Merge #13527: policy: Remove promiscuousmempoolflags
ClosedPublic
Actions

Authored by markblundeberg on May 21 2019, 06:14.

Details

Reviewers

jasonbcox
Fabien
deadalnix

Group Reviewers

Restricted Project

Commits

rSTAGINGc08418054e83: Merge #13527: policy: Remove promiscuousmempoolflags
rABCc08418054e83: Merge #13527: policy: Remove promiscuousmempoolflags

Summary

PR13527 backport https://github.com/bitcoin/bitcoin/pull/13527/files
faa24441ec047ec336b86f586016b9d318c1c0ad policy: Remove promiscuousmempoolflags (MarcoFalke)

Pull request description:

It seems odd to clutter validation code with features that can only ever be used for testing (testnet or regtest). Removing that test-only code makes the mempool logic less painful to understand and easier to reason about when changed or refactored in the future.

ABC note:

promiscuousmempoolflags was originally added for segwit tests (#8149).
adjusted dersig failure message -> mandatory
due to large changes in D2120, the feature_cltv.py changes have been massaged into the appropriate place

Depends on D3079 and D3095

Test Plan

make check
test_runner.py

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

markblundeberg created this revision.May 21 2019, 06:14

Herald added a reviewer: Restricted Project. · View Herald TranscriptMay 21 2019, 06:14

Herald added a reviewer: deadalnix. · View Herald Transcript

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

markblundeberg added inline comments.May 21 2019, 06:18

test/functional/feature_cltv.py
180 ↗	(On Diff #8771)	I notice there is some bizarre whitespace character here marked with ! ... wonder how that happened.

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B5920: Diff 8771.May 21 2019, 06:29

deadalnix requested changes to this revision.May 22 2019, 00:07

deadalnix added inline comments.

src/validation.cpp
732 ↗	(On Diff #8771)	You can probably merge this with what precedes and make it const while you are at it.
test/functional/feature_cltv.py
156 ↗	(On Diff #8771)	It does looks like this is the test to be replaced.
180 ↗	(On Diff #8771)	Probably a misencoded character.

This revision now requires changes to proceed.May 22 2019, 00:07

markblundeberg added inline comments.May 22 2019, 02:28

src/validation.cpp
732 ↗	(On Diff #8771)	Sounds good :)
test/functional/feature_cltv.py
156 ↗	(On Diff #8771)	Yeah I thought so at first, but `spendtx` here is misnamed -- really should be called "fundtx" since the CLTV violation is in the scriptPubKey. The actual tx that executes the bad CLTV is then `rejectedtx` below. Confusing! The Core version of this (like we had prior to D2120) has it in the scriptSig of this transaction, so the `spendtx` name made more sense. Maybe a new diff should be opened to clarify this stuff, it confused me for a while. I guess the comment here is completely wrong too!