Page MenuHomePhabricator

[qa] Use proper fee accounting in mempool_limit
ClosedPublic

Authored by schancel on Jul 20 2018, 19:57.

Details

Summary

As per title

Test Plan
./test/functional/test_runner.py

Diff Detail

Repository
rABC Bitcoin ABC
Branch
fix-mempool-limit
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 2955
Build 4010: Bitcoin ABC Buildbot (legacy)
Build 4009: arc lint + arc unit

Event Timeline

jasonbcox requested changes to this revision.Jul 20 2018, 20:57
jasonbcox added a subscriber: jasonbcox.
jasonbcox added inline comments.
test/functional/bip68-sequence.py
232 ↗(On Diff #4363)

wasn't this 10 before? any reason to go to 100 or typo?

also, this appears to be added to the wrong file. not sure how this value is being accessed from the other test.

test/functional/mempool_limit.py
14 ↗(On Diff #4363)

Any reason to not add this to test_framework/util.py? If you do this, maybe rename it to create_lots_of_big_transactions_with_opreturn

21 ↗(On Diff #4363)

Isn't this adding 65536 bytes? How does this pass?

26 ↗(On Diff #4363)

Please rename t. I don't know what this means.

32 ↗(On Diff #4363)

transaciton -> transaction

72 ↗(On Diff #4363)

is the added 10 supposed to be fee_multiplier?

This revision now requires changes to proceed.Jul 20 2018, 20:57
test/functional/bip68-sequence.py
232 ↗(On Diff #4363)

Looks like this is already part of D1561

test/functional/bip68-sequence.py
232 ↗(On Diff #4363)

I missed removing this from the diff. It got picked up when I cherry-picked for some reason.

test/functional/mempool_limit.py
14 ↗(On Diff #4363)

So, mininode depends on util, so util cannot depend on mininode. Mininode has all these data types in it. I need to refactor the entire test framework layout at some point.

21 ↗(On Diff #4363)

What do you mean? regtest allows nonstandard transactions. This is what this function has always done in the past.

72 ↗(On Diff #4363)

The entire thing is a fee_multiplier. It's used in the function as such.

Fix mempool_limit and address feedback

Address feedback about create_lots_of_big_transactions

This revision is now accepted and ready to land.Jul 25 2018, 20:36
This revision was automatically updated to reflect the committed changes.