Page MenuHomePhabricator

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

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

Repository
rABC Bitcoin ABC
Branch
adjust-weight
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 3727
Build 5529: Bitcoin ABC Buildbot (legacy)
Build 5528: arc lint + arc unit

Event Timeline

schancel created this revision.Oct 12 2018, 21:03
Herald added a reviewer: Restricted Project. · View Herald TranscriptOct 12 2018, 21:03
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 marked an inline comment as done.Oct 15 2018, 17:44
schancel added inline comments.
src/primitives/transaction.cpp
133 ↗(On Diff #5420)

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.

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 ↗(On Diff #5420)

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 ↗(On Diff #5420)

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 ↗(On Diff #5420)

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 ↗(On Diff #5420)

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 ↗(On Diff #5420)

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

jasonbcox added inline comments.Oct 25 2018, 21:27
src/primitives/transaction.cpp
133 ↗(On Diff #5420)

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

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

Rebase fix conflicts

jasonbcox requested changes to this revision.Nov 12 2018, 22:13
This revision now requires changes to proceed.Nov 12 2018, 22:13