Page MenuHomePhabricator

[qa] allow assert_fee to have a bit more wiggleroom
ClosedPublic

Authored by schancel on Oct 1 2018, 22:14.

Details

Summary

As per title

Test Plan
make check && ./test/functional/test_runner.py

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

schancel created this revision.Oct 1 2018, 22:14
Herald added a reviewer: Restricted Project. · View Herald TranscriptOct 1 2018, 22:14
deadalnix requested changes to this revision.Oct 2 2018, 11:24
deadalnix added a subscriber: deadalnix.

Please update the description to explain why this needs to happen.

This revision now requires changes to proceed.Oct 2 2018, 11:24
schancel abandoned this revision.Oct 5 2018, 00:47

I actually ended up fixing a different cause of the failure.

schancel reclaimed this revision.Oct 5 2018, 01:11
This revision now requires changes to proceed.Oct 5 2018, 01:11
schancel added a comment.Oct 5 2018, 01:27

Okay, so for fundrawtransaction, when fees are dropped, the first transaction sent in a series of fee checks is with a feerate of 100sat/kb. This results in a fee of 0.00000202. This is ceiled so as to not be under the minimum relay fee. It then proceeds to calculate expected fees based on multiples of this due to the dummy signatures that get added during fundraw transaction.

However, the full fee was 0.000002016. So when it tries another transaction with a 10x feerate it expects 0.00002020 due to the previous truncation -- it then fails.

Something like this patch is needed to deal with that kind of problem. This particular method may not be best.

jasonbcox requested changes to this revision.Oct 5 2018, 18:13
jasonbcox added a subscriber: jasonbcox.
jasonbcox added inline comments.
test/functional/test_framework/util.py
28 ↗(On Diff #5216)

Depending on how rigid the +/- 10 range is, you could include it as a parameter to this function and allow the individual tests to determine the tolerance.

34 ↗(On Diff #5216)

Update this comment and make it reflect how +/- 10 was chosen

schancel updated this revision to Diff 5294.Oct 5 2018, 19:10

Update for clarity

jasonbcox accepted this revision.Oct 5 2018, 19:23
deadalnix accepted this revision.Oct 8 2018, 11:56
This revision is now accepted and ready to land.Oct 8 2018, 11:56
This revision was automatically updated to reflect the committed changes.