Page MenuHomePhabricator

p2sh txns added to the mempool
ClosedPublic

Authored by hanchon on Jun 20 2017, 18:12.

Details

Reviewers
deadalnix
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Project
Commits
rSTAGING29dbc00975d4: p2sh txns added to the mempool
rABC29dbc00975d4: p2sh txns added to the mempool
Summary

p2sh txns were added to the mempool, for the task https://reviews.bitcoinabc.org/T47

Test Plan

python3 rpc-tests.py mempool_p2sh_txn.py

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Owners added a reviewer: Restricted Owners Package.Jun 20 2017, 18:12
qa/rpc-tests/abc-p2p-fullblocktest.py
28 ↗(On Diff #592)

Should MAX_STANDARD_TX_SIGOPS be moved to the cdefs file? The value is defined in the policy.h file

Because this end up not that much related to ABC (this rule needs to be respected regardless of ABC acitvation) this would benefit from being moved to one of the mempool related rpc test (or if none fit the bill, a new RPC test).

qa/rpc-tests/abc-p2p-fullblocktest.py
28 ↗(On Diff #592)

I think it makes sense.

Reverted changes on abc-p2p-fullblocktest.py
Created new test named mempool_p2sh_txn.py derived from p2p-fullblocktest
The new test was added to the qa/pull-tester/rpc-tests.py
MAX_STANDARD_TX_SIGOPS was declared in the qa/rpc-tests/test-framework/cdefs.py file

Please modify Test Plan to use the new name of the test (use Edit Revision to change).

deadalnix requested changes to this revision.Jun 21 2017, 08:36

The test needs to be renamed, and, as per @freetrader mentioned, the test plan needs to be updated to reflect the new test name. Except that, this is good.

qa/pull-tester/rpc-tests.py
160 ↗(On Diff #595)

Other tests are using dashes in names. We should do the same. We also may want to add more tests in there in the future, because what goes in the mempool is very much untested.

This revision now requires changes to proceed.Jun 21 2017, 08:36
hanchon added inline comments.
qa/pull-tester/rpc-tests.py
160 ↗(On Diff #595)

The mempool tests are using "_" instead of "-". (mempool_limit.py, memepool_packages.py, memppol_reorg.py,memepool_resurrect_test.py and mempool_spendcoinbase.py) . Should I change the name of this test to use dashes?

hanchon edited edge metadata.

Test renamed to mempool-accept-txn.py
Test name also was edited in the qa/pull-tester/rpc-tests.py file
Added missing end line at the end of the abc-p2p-fullblocktest.py file (that line was not copied when copy-pasted the original abc-p2p-fullblocktest.py code to revert the changes)

This revision is now accepted and ready to land.Jun 21 2017, 13:18
This revision was automatically updated to reflect the committed changes.