Page MenuHomePhabricator

Begin using billable txn size for wallet and mining
Changes PlannedPublic

Authored by schancel on Sep 24 2018, 00:05.

Details

Reviewers
jasonbcox
deadalnix
Group Reviewers
Restricted Project
Summary

Currently, we use the full transaction size for a minimum fee calculation.
This commit changes the behavior to charge different effective rates for
portions of the transaction.

Depends on D1798, D1800, D1803, D1804, D1860

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

Diff Detail

Branch
billable-size
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 3401
Build 4879: arc lint + arc unit

Event Timeline

Remote release note change

Don't touch release-notes

schancel retitled this revision from Introduce a billable size that is independent from the total size to Introduce a tx billable size that is independent from the total size.Sep 24 2018, 17:34

Rebased auxilliary changes away

jasonbcox requested changes to this revision.Sep 25 2018, 00:30
jasonbcox added a subscriber: jasonbcox.

Requesting changes because some errant line deletions in files that are otherwise untouched.

Otherwise, the diff looks good ๐Ÿ‘

src/wallet/rpcwallet.cpp
3410 โ†—(On Diff #5016)

Unnecessary for this diff. Please remove.

src/wallet/wallet.cpp
2960 โ†—(On Diff #5016)

duplicate space

test/functional/abc-high_priority_transaction.py
16 โ†—(On Diff #5016)

remove

test/functional/pruning.py
17 โ†—(On Diff #5016)

remove

This revision now requires changes to proceed.Sep 25 2018, 00:30
deadalnix requested changes to this revision.Sep 27 2018, 18:41
deadalnix added a subscriber: deadalnix.

You really should split that up. One to keep track of the billable size in the mempool + unit tests that check that the size is tracked properly with acsendent and descendent txns. This part seems to be mostly taken care of, but would benefit from being unit tested.

The second one to actually start using the billable size. I'm not sure about this part, tbh, and having it isolated from the rest of the change would make review easier.

This revision now requires changes to proceed.Sep 27 2018, 18:41
schancel retitled this revision from Introduce a tx billable size that is independent from the total size to Implement billable size usage in mining, relaying, and wallet transaction creation.
deadalnix requested changes to this revision.Oct 5 2018, 10:30

Considering there are changes planned in dependencies, I'll put that back on your queue.

This revision now requires changes to proceed.Oct 5 2018, 10:30

Include test here, rather than next item in stack

Fix billablesize tracking and assertions

schancel marked an inline comment as done.
schancel added inline comments.
src/test/mempool_tests.cpp
455 โ†—(On Diff #5403)

These need to be moved to the next commit in the stack.

Rebase + Move mempool_tests to proper commits

schancel retitled this revision from Implement billable size usage in mining, relaying, and wallet transaction creation to Begin using billable txn size for wallet and mining.Oct 22 2018, 21:02