Page MenuHomePhabricator
Differential D1927

Use a modified size for transaction minimum fee calculation.
Needs RevisionPublic
Actions

Authored by schancel on Oct 12 2018, 21:03.

Details

Reviewers
jasonbcox
Group Reviewers
Restricted Project
Summary

Modify BillableSize to just a modified calculation. Additionally,
drop DEFAULT_MIN_RELAY_TX_FEE_PER_KB to 100sat/kb.

Depends on D1801

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

Diff Detail

Event Timeline

schancel created this revision.Oct 12 2018, 21:03
Herald added a reviewer: Restricted Project. · View Herald TranscriptOct 12 2018, 21:03
teamcity added a subscriber: teamcity.Oct 12 2018, 21:04

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

schancel updated this revision to Diff 5402.Oct 12 2018, 21:07

Rebase without test here.

schancel updated this revision to Diff 5404.Oct 12 2018, 21:09

Remove fixes to txmempooll.

schancel edited the summary of this revision. (Show Details)Oct 12 2018, 21:10
schancel added a parent revision: D1801: Begin using billable txn size for wallet and mining.
teamcity added a comment.Oct 12 2018, 21:14

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

teamcity added a comment.Oct 12 2018, 21:14

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

Harbormaster completed remote builds in B3591: Diff 5400.Oct 12 2018, 21:15
teamcity added a comment.Oct 12 2018, 21:15

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

teamcity added a comment.Oct 12 2018, 21:24

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

Harbormaster failed remote builds in B3593: Diff 5402!Oct 12 2018, 21:24
teamcity added a comment.Oct 12 2018, 21:24

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

Harbormaster completed remote builds in B3595: Diff 5404.Oct 12 2018, 21:24
schancel updated this revision to Diff 5420.Oct 15 2018, 17:42

Rebase

teamcity added a comment.Oct 15 2018, 17:43

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

schancel marked an inline comment as done.Oct 15 2018, 17:44
schancel added inline comments.
src/primitives/transaction.cpp
133

I'm not sure the above calculation is ideal. Let's talk about what we want here. With the fees dropped, this makes a 1 input + 2 output transaction cost 1sat/byte roughly. However, I don't like the linear formula, and the 100% return rate when spending a UTXO.

teamcity added a comment.Oct 15 2018, 17:51

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

Harbormaster completed remote builds in B3602: Diff 5420.Oct 15 2018, 17:51
jasonbcox requested changes to this revision.Oct 16 2018, 18:22
jasonbcox added a subscriber: jasonbcox.
jasonbcox added inline comments.
doc/release-notes.md
11

Either this comment is reading funny compared to the comments in GetBillableSize() or this should say 179 per output instead of 1790. Can you clarify?

src/primitives/transaction.cpp
125

The comment refers to 179 bytes. Where does the * 10 multiplier come from? I assume it's derived from the 100 sat/kB value, but this calculation should derive from that as well.

133

Since spending a UTXO doesn't increase the UTXO set size, we should be safe here. I think the linear formula will work for now and can be optimized as we continue to improve the mempool and other more critical paths.

Incremental improvements :)

This revision now requires changes to proceed.Oct 16 2018, 18:22
schancel marked 2 inline comments as done.Oct 20 2018, 03:07
schancel added inline comments.
src/primitives/transaction.cpp
125

Yeah, sorry the comment was outdated when I dropped the fees to 100 sat/byte, I upped the total size of the UTXO to 1790.

I don't really like this formula at all though. I want to discuss it more.

133

It's pressure on the wallet ecosystem everytime this changes.

schancel updated this revision to Diff 5506.Oct 22 2018, 21:20

Rebase

teamcity added a comment.Oct 22 2018, 21:22

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

teamcity added a comment.Oct 22 2018, 21:30

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

Harbormaster completed remote builds in B3661: Diff 5506.Oct 22 2018, 21:30
jasonbcox added inline comments.Oct 25 2018, 21:27
src/primitives/transaction.cpp
133

It should only be pressure on wallets if the minfee increases. Decreases in the acceptable minfee can be adopted by wallets at any time.

schancel added a comment.Oct 29 2018, 21:31
This comment was removed by schancel.
schancel updated this revision to Diff 5593.Oct 30 2018, 23:20

Rebase fix conflicts

teamcity added a comment.Oct 30 2018, 23:22

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

teamcity added a comment.Oct 30 2018, 23:30

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

Harbormaster completed remote builds in B3727: Diff 5593.Oct 30 2018, 23:30
jasonbcox requested changes to this revision.Nov 12 2018, 22:13
This revision now requires changes to proceed.Nov 12 2018, 22:13

Revision Contents
Changeset List

PathSize
doc/
4 lines
src/
policy/
2 lines
primitives/
17 lines
test/
12 lines
2 lines
2 lines
test/
functional/
2 lines
test_framework/
12 lines

Diff 5420

View Options

doc/release-notes.md

Loading...
View Options

src/policy/policy.h

Loading...
View Options

src/primitives/transaction.cpp

Loading...
View Options

src/test/mempool_tests.cpp

Loading...
View Options

src/test/transaction_tests.cpp

Loading...
View Options

src/validation.h

Loading...
View Options

test/functional/fundrawtransaction.py

Loading...
View Options

test/functional/test_framework/mininode.py

Loading...