Page MenuHomePhabricator
Differential D1486

Use a modified size for transaction minimum fee calculation.
AbandonedPublic
Actions

Authored by schancel on Jun 7 2018, 05:18.

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 for the additional (minimum) of
179 bytes it would take to spend any newly created outputs.

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

Diff Detail

Repository
rABC Bitcoin ABC
Branch
arcpatch-D1486
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 2762
Build 3634: Bitcoin ABC Buildbot (legacy)
Build 3633: arc lint + arc unit

Event Timeline

schancel created this revision.Jun 7 2018, 05:18
teamcity added a comment.Jun 7 2018, 05:19

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

teamcity added a comment.Jun 7 2018, 05:55

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

Harbormaster completed remote builds in B2714: Diff 4081.Jun 7 2018, 05:55
schancel updated this revision to Diff 4084.Jun 7 2018, 11:54

Use FromHex helper

teamcity added a comment.Jun 7 2018, 11:56

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

teamcity added a comment.Jun 7 2018, 12:30

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

Harbormaster completed remote builds in B2717: Diff 4084.Jun 7 2018, 12:30
jasonbcox mentioned this in D1480: Tweak UTXO weight based agreed suggested costs.Jun 8 2018, 23:10
jasonbcox requested changes to this revision.Jun 8 2018, 23:26

Everything else looks good. I assume the reason you aren't merging in D1480 is that that particular change fails tests and may be it's own can of worms, yes?

src/test/transaction_tests.cpp
768–769 ↗(On Diff #4084)

I'm finding this hard to read with the short names. Please do (or similar):
bs -> billableSize
s -> size

This revision now requires changes to proceed.Jun 8 2018, 23:26
schancel added inline comments.Jun 12 2018, 14:48
src/txmempool.cpp
897 ↗(On Diff #4084)

The previous GetTxSize() cached the size internally to the mempool entry. This does not. I am curious as to if I should make the mempool entry cache this data.

schancel updated this revision to Diff 4125.Jun 15 2018, 11:20

Address feeback about tests

teamcity added a comment.Jun 15 2018, 11:21

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

schancel planned changes to this revision.Jun 15 2018, 11:25

I still need to add caching for the billing size.

teamcity added a comment.Jun 15 2018, 11:41

Build Bitcoin-ABC :: Diffs :: Bitcoin-ABC Diff Testing failed.

Harbormaster failed remote builds in B2742: Diff 4125!Jun 15 2018, 11:41
schancel updated this revision to Diff 4155.Jun 23 2018, 09:28

Fix up the commit to properly cache billable size result

teamcity added a comment.Jun 23 2018, 09:29

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

teamcity added a comment.Jun 23 2018, 10:03

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

Harbormaster completed remote builds in B2762: Diff 4155.Jun 23 2018, 10:03
jasonbcox accepted this revision.Jun 25 2018, 17:08
jasonbcox added inline comments.
src/wallet/wallet.cpp
2921

remove second space after // the

This revision is now accepted and ready to land.Jun 25 2018, 17:08
schancel added a child revision: D1480: Tweak UTXO weight based agreed suggested costs.Jul 2 2018, 17:36
schancel added a parent revision: D1561: [qa] change bip68 sequence to use calculate_fee.Jul 18 2018, 17:13
schancel updated this revision to Diff 4353.Jul 18 2018, 19:15

Rebase

teamcity added a comment.Jul 18 2018, 19:17

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

Harbormaster failed remote builds in B2903: Diff 4353!Jul 18 2018, 19:56
teamcity added a comment.Jul 18 2018, 19:56

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

schancel updated this revision to Diff 4355.Jul 18 2018, 21:02

rebase

teamcity added a comment.Jul 18 2018, 21:05

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

teamcity added a comment.Jul 18 2018, 21:42

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

Harbormaster completed remote builds in B2905: Diff 4355.Jul 18 2018, 21:42
schancel updated this revision to Diff 4358.Jul 18 2018, 22:15

Rebase

teamcity added a comment.Jul 18 2018, 22:16

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

teamcity added a comment.Jul 18 2018, 22:55

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

Harbormaster completed remote builds in B2908: Diff 4358.Jul 18 2018, 22:55
jasonbcox accepted this revision.Jul 19 2018, 02:22
schancel added a child revision: D1581: Update CTxMemPoolEntry and BlockAssembler to billing cutoffs.Jul 19 2018, 20:28
schancel removed a child revision: D1480: Tweak UTXO weight based agreed suggested costs.
schancel added a parent revision: D1582: [qa] Use proper fee accounting in mempool_limit.Jul 20 2018, 19:59
schancel updated this revision to Diff 4420.Jul 25 2018, 18:43

Include billable_size in this diff

teamcity added a comment.Jul 25 2018, 18:51

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

Harbormaster failed remote builds in B2957: Diff 4420!Jul 25 2018, 19:28
teamcity added a comment.Jul 25 2018, 19:28

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

jasonbcox accepted this revision.Jul 25 2018, 20:35
jasonbcox added inline comments.
src/primitives/transaction.cpp
114 ↗(On Diff #4420)

change -> charge?

schancel updated this revision to Diff 4453.Jul 26 2018, 21:33

Rebase

teamcity added a comment.Jul 26 2018, 21:33

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

schancel updated this revision to Diff 4454.Jul 26 2018, 21:33

Rebase

teamcity added a comment.Jul 26 2018, 21:33

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

schancel updated this revision to Diff 4456.Jul 26 2018, 21:55
schancel added a child revision: D1480: Tweak UTXO weight based agreed suggested costs.

Rebase

teamcity added a comment.Jul 26 2018, 21:58

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

teamcity added a comment.Jul 26 2018, 22:11

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

Harbormaster failed remote builds in B2982: Diff 4454!Jul 26 2018, 22:11
Harbormaster failed remote builds in B2981: Diff 4453!
teamcity added a comment.Jul 26 2018, 22:12

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

Harbormaster completed remote builds in B2984: Diff 4456.Jul 26 2018, 22:36
teamcity added a comment.Jul 26 2018, 22:36

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

schancel abandoned this revision.Aug 6 2018, 17:49

Revision Contents
Changeset List

PathSize
doc/
3 lines
src/
primitives/
4 lines
18 lines
test/
20 lines
4 lines
3 lines
10 lines
wallet/
12 lines
test/
functional/
2 lines
4 lines
test_framework/
5 lines
2 lines
12 lines

Diff 4155

View Options

doc/release-notes.md

Loading...
View Options

src/primitives/transaction.h

Loading...
View Options

src/primitives/transaction.cpp

Loading...
View Options

src/test/transaction_tests.cpp

Loading...
View Options

src/txmempool.h

Loading...
View Options

src/txmempool.cpp

Loading...
View Options

src/validation.cpp

Loading...
View Options

src/wallet/wallet.cpp

Loading...
View Options

test/functional/fundrawtransaction.py

Loading...
View Options

test/functional/mempool_limit.py

Loading...
View Options

test/functional/test_framework/mininode.py

Loading...
View Options

test/functional/test_framework/test_node.py

Loading...
View Options

test/functional/wallet.py

Loading...