New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid selecting uneconomic UTXO during transaction creation #473

Merged
merged 3 commits into from Jun 2, 2017

Conversation

Projects
None yet
2 participants
@dexX7
Copy link
Member

dexX7 commented May 30, 2017

  1. #396 introduced a workaround to bloat transactions, to get around a "sigops per byte limit". This limit was removed by bitcoin#8365, and the workaround actually makes the situation worse, therefore the workaround is removed.

  2. Failures during the wallet transaction are logged.

  3. To avoid bloating transactions while lowering the effective fee, inputs are filtered during the coin selection, and uneconomic inputs are excluded. This is based on the estimated fee used for the transaction creation. Instead of selecting an amount based on a magic number, the amount to select for the transaction creation is also based on the estimated fee, used to create the transaction.

@dexX7 dexX7 added this to the Next release milestone May 30, 2017

@dexX7 dexX7 changed the title Remove sigops workaround for wallet transactions Avoid selecting uneconomic UTXO during transaction creation May 31, 2017

Use estimated fees to filter and create Omni wallet transactions
To avoid bloating transactions while lowering the effective fee, inputs are filtered during the coin selection, and uneconomic inputs are excluded. This is based on the estimated fee used for the transaction creation.

Instead of selecting an amount based on a magic number, the amount to select for the transaction creation is also based on the estimated fee, used to create the transaction.

@dexX7 dexX7 force-pushed the dexX7:0.2-wallet-remove-sigops-workaround branch from 3859b5f to bbd2ee8 Jun 1, 2017

@zathras-crypto

This comment has been minimized.

Copy link

zathras-crypto commented Jun 2, 2017

Copying slack commentary here for historical purposes...

Alrighty @dexx, I ran some testing across your code changes for transaction creation as promised 🙂

What I was looking for was to confirm the expected behaviour of excluding outputs when they became uneconomical. To do this I made some tests that sprinkled dust over an address (seeded it with a bunch of dust-ish outputs) along with seeding a single good sized UTXO. I then used varying fee/kb values to see whether those dust-ish outputs would be used when creating a transaction.

My math for expected results was:

  • 1,000 satoshis/kb : An output with a value of 2,730 satoshis & a size of ~150 bytes would cost 150 satoshis to include. 150 < 2730 so it's economical to include it.
  • 5,000 satoshis/kb : An output with a value of 2,730 satoshis & a size of ~150 bytes would cost 750 satoshis to include. 750 < 2730 so it's economical to include it.
  • 10,000 satoshis/kb : An output with a value of 2,730 satoshis & a size of ~150 bytes would cost 1500 satoshis to include. 1500 < 2730 so it's economical to include it.
  • 20,000 satoshis/kb : An output with a value of 2,730 satoshis & a size of ~150 bytes would cost 3000 satoshis to include. 3000 > 2730 so it's not economical to include it.

We would thus expect that for fee values of 1,000/kb, 5,000/kb and 10,000/kb that these dust-ish outputs would be included in the transaction. However at a fee rate of 20,000/kb we should expect that these dust-ish outputs would not be included in the transaction.

All the results were just as expected (I'll post in a snippet in a sec) so at least as far as this kind of testing goes I think the change is all good 🙂 I only tested with explicit paytxfee values because I'm not sure yet how to manipulate confirm targets for testing but this should give some more confidence (perhaps @adam could run this change at OW for further confidence?)

Test results with 1,000 satoshis/kb fee:

2017-06-02 00:44:26 SelectCoins: sender: n11Hotb9rMHSTUxgqhux2bnP13rmJLFQ9T, outpoint: 95d7f294e74ccf90926813d77232051f397f8fdadb65af2566e436d41cd1ca28:0, value: 2730
2017-06-02 00:44:26 SelectCoins: sender: n11Hotb9rMHSTUxgqhux2bnP13rmJLFQ9T, outpoint: 95d7f294e74ccf90926813d77232051f397f8fdadb65af2566e436d41cd1ca28:2, value: 2730
2017-06-02 00:44:26 SelectCoins: sender: n11Hotb9rMHSTUxgqhux2bnP13rmJLFQ9T, outpoint: 95d7f294e74ccf90926813d77232051f397f8fdadb65af2566e436d41cd1ca28:3, value: 2730
2017-06-02 00:44:26 SelectCoins: sender: n11Hotb9rMHSTUxgqhux2bnP13rmJLFQ9T, outpoint: 95d7f294e74ccf90926813d77232051f397f8fdadb65af2566e436d41cd1ca28:4, value: 2730
2017-06-02 00:44:26 SelectCoins: sender: n11Hotb9rMHSTUxgqhux2bnP13rmJLFQ9T, outpoint: 95d7f294e74ccf90926813d77232051f397f8fdadb65af2566e436d41cd1ca28:5, value: 2730
2017-06-02 00:44:26 SelectCoins: sender: n11Hotb9rMHSTUxgqhux2bnP13rmJLFQ9T, outpoint: c00991896019e2ba39032bf04d6e9d624ed1cee7d4718c3aac6d40a4c83684c8:0, value: 50000000
2017-06-02 00:44:26 WalletTxBuilder: CTransaction(hash=1f39e1c678, ver=1, vin.size=6, vout.size=2, nLockTime=106)
    CTxIn(COutPoint(95d7f294e7, 0), scriptSig=483045022100ddd7ef08e90e, nSequence=4294967294)
    CTxIn(COutPoint(95d7f294e7, 2), scriptSig=473044022077d5bff03b8938, nSequence=4294967294)
    CTxIn(COutPoint(95d7f294e7, 3), scriptSig=47304402205c36abee20d672, nSequence=4294967294)
    CTxIn(COutPoint(95d7f294e7, 4), scriptSig=47304402200fac1b5639aaf5, nSequence=4294967294)
    CTxIn(COutPoint(95d7f294e7, 5), scriptSig=473044022025ef66ef09196b, nSequence=4294967294)
    CTxIn(COutPoint(c009918960, 0), scriptSig=483045022100e89c5979cf80, nSequence=4294967294)
    CTxOut(nValue=0.50012678, scriptPubKey=76a914d5c6539e8a3b568f38bff4eb)
    CTxOut(nValue=0.00000000, scriptPubKey=6a216f6d6e69000000320100010000)
; nFeeRet = 972



Test results with 5,000 satoshis/kb fee:

2017-06-02 00:46:23 SelectCoins: sender: moNZmBmQZ46fyH3sPQ19pxnh16TGuDrJD6, outpoint: 230e71d8580ccc1ac6ed73a5bf3d25d9ef15922a9af212c75d793f7cbb42a833:0, value: 2730
2017-06-02 00:46:23 SelectCoins: sender: moNZmBmQZ46fyH3sPQ19pxnh16TGuDrJD6, outpoint: 230e71d8580ccc1ac6ed73a5bf3d25d9ef15922a9af212c75d793f7cbb42a833:1, value: 2730
2017-06-02 00:46:23 SelectCoins: sender: moNZmBmQZ46fyH3sPQ19pxnh16TGuDrJD6, outpoint: 230e71d8580ccc1ac6ed73a5bf3d25d9ef15922a9af212c75d793f7cbb42a833:2, value: 2730
2017-06-02 00:46:23 SelectCoins: sender: moNZmBmQZ46fyH3sPQ19pxnh16TGuDrJD6, outpoint: 230e71d8580ccc1ac6ed73a5bf3d25d9ef15922a9af212c75d793f7cbb42a833:4, value: 2730
2017-06-02 00:46:23 SelectCoins: sender: moNZmBmQZ46fyH3sPQ19pxnh16TGuDrJD6, outpoint: 230e71d8580ccc1ac6ed73a5bf3d25d9ef15922a9af212c75d793f7cbb42a833:5, value: 2730
2017-06-02 00:46:23 SelectCoins: sender: moNZmBmQZ46fyH3sPQ19pxnh16TGuDrJD6, outpoint: a8a3d265af72ec957b5838b6865aa39094a334ed9e5179cc4df607ef26d5d434:0, value: 50000000
2017-06-02 00:46:23 WalletTxBuilder: CTransaction(hash=d441d4af5d, ver=1, vin.size=6, vout.size=2, nLockTime=106)
    CTxIn(COutPoint(230e71d858, 0), scriptSig=483045022100913cc62adc46, nSequence=4294967294)
    CTxIn(COutPoint(230e71d858, 1), scriptSig=47304402202a808e35f054ba, nSequence=4294967294)
    CTxIn(COutPoint(230e71d858, 2), scriptSig=4730440220236ba9375516a4, nSequence=4294967294)
    CTxIn(COutPoint(230e71d858, 4), scriptSig=483045022100e1442a595457, nSequence=4294967294)
    CTxIn(COutPoint(230e71d858, 5), scriptSig=4730440220735053292a5243, nSequence=4294967294)
    CTxIn(COutPoint(a8a3d265af, 0), scriptSig=4730440220501ad9afa31f48, nSequence=4294967294)
    CTxOut(nValue=0.00000000, scriptPubKey=6a216f6d6e69000000320100010000)
    CTxOut(nValue=0.50008780, scriptPubKey=76a914562adea0e7681111179d2c40)
; nFeeRet = 4870



Test results with 10,000 satoshis/kb fee:

2017-06-02 00:47:08 SelectCoins: sender: n4fTzEFEnLAMb5CEPbpyzMdeYCsSw1YJdj, outpoint: 585cb2656909ee403a790ee312dc541a752898fc48848f50ff03d38159019567:0, value: 2730
2017-06-02 00:47:08 SelectCoins: sender: n4fTzEFEnLAMb5CEPbpyzMdeYCsSw1YJdj, outpoint: 585cb2656909ee403a790ee312dc541a752898fc48848f50ff03d38159019567:2, value: 2730
2017-06-02 00:47:08 SelectCoins: sender: n4fTzEFEnLAMb5CEPbpyzMdeYCsSw1YJdj, outpoint: 585cb2656909ee403a790ee312dc541a752898fc48848f50ff03d38159019567:3, value: 2730
2017-06-02 00:47:08 SelectCoins: sender: n4fTzEFEnLAMb5CEPbpyzMdeYCsSw1YJdj, outpoint: 585cb2656909ee403a790ee312dc541a752898fc48848f50ff03d38159019567:4, value: 2730
2017-06-02 00:47:08 SelectCoins: sender: n4fTzEFEnLAMb5CEPbpyzMdeYCsSw1YJdj, outpoint: 585cb2656909ee403a790ee312dc541a752898fc48848f50ff03d38159019567:5, value: 2730
2017-06-02 00:47:08 SelectCoins: sender: n4fTzEFEnLAMb5CEPbpyzMdeYCsSw1YJdj, outpoint: c5ff63a2916a14e5afc121c5bc171ec5512f01e1aed132b1faeaa9fa8920cfed:1, value: 50000000
2017-06-02 00:47:08 WalletTxBuilder: CTransaction(hash=7bfe09c22a, ver=1, vin.size=6, vout.size=2, nLockTime=106)
    CTxIn(COutPoint(585cb26569, 0), scriptSig=47304402203dcb10e8db018a, nSequence=4294967294)
    CTxIn(COutPoint(585cb26569, 2), scriptSig=47304402205e7771c9a108e4, nSequence=4294967294)
    CTxIn(COutPoint(585cb26569, 3), scriptSig=47304402201c323c90fdd6cb, nSequence=4294967294)
    CTxIn(COutPoint(585cb26569, 4), scriptSig=483045022100fa24f4fcd96c, nSequence=4294967294)
    CTxIn(COutPoint(585cb26569, 5), scriptSig=47304402206d5dbeaae00bf6, nSequence=4294967294)
    CTxIn(COutPoint(c5ff63a291, 1), scriptSig=483045022100cd8792bf62bc, nSequence=4294967294)
    CTxOut(nValue=0.50003920, scriptPubKey=76a914fde701c565b77e2517b36bcb)
    CTxOut(nValue=0.00000000, scriptPubKey=6a216f6d6e69000000320100010000)
; nFeeRet = 9730



Test results with 20,000 satoshis/kb fee:

2017-06-02 00:48:50 SelectCoins: output value below economic threshold: d7f0b622f94e5205674791a5b9a50ad5ab2476a30f8d5db775ad7acda79f633b:0, value: 2730
2017-06-02 00:48:50 SelectCoins: output value below economic threshold: d7f0b622f94e5205674791a5b9a50ad5ab2476a30f8d5db775ad7acda79f633b:1, value: 2730
2017-06-02 00:48:50 SelectCoins: output value below economic threshold: d7f0b622f94e5205674791a5b9a50ad5ab2476a30f8d5db775ad7acda79f633b:2, value: 2730
2017-06-02 00:48:50 SelectCoins: output value below economic threshold: d7f0b622f94e5205674791a5b9a50ad5ab2476a30f8d5db775ad7acda79f633b:4, value: 2730
2017-06-02 00:48:50 SelectCoins: output value below economic threshold: d7f0b622f94e5205674791a5b9a50ad5ab2476a30f8d5db775ad7acda79f633b:5, value: 2730
2017-06-02 00:48:50 SelectCoins: sender: mssNCrJZJLbyEs14cd5tFHwz7RVwShwuPJ, outpoint: bcf8996773ee250ccc4c58324b51d3f2eae954762dbfa94aacda04c6a5cf44f2:1, value: 50000000
2017-06-02 00:48:50 WalletTxBuilder: CTransaction(hash=38d8ddb74e, ver=1, vin.size=1, vout.size=2, nLockTime=106)
    CTxIn(COutPoint(bcf8996773, 1), scriptSig=473044022064dc2586e7bd2a, nSequence=4294967294)
    CTxOut(nValue=0.00000000, scriptPubKey=6a216f6d6e69000000320100010000)
    CTxOut(nValue=0.49995280, scriptPubKey=76a914877dc9b6dab5b0787b3b0a99)
; nFeeRet = 4720
@zathras-crypto

This comment has been minimized.

Copy link

zathras-crypto commented Jun 2, 2017

Reviewed and tested, OK to merge - great work @dexX7 thanks very much :)

@dexX7 dexX7 merged commit bbd2ee8 into OmniLayer:develop Jun 2, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

dexX7 added a commit that referenced this pull request Jun 2, 2017

Merge #473: Avoid selecting uneconomic UTXO during transaction creation
bbd2ee8 Use estimated fees to filter and create Omni wallet transactions (dexX7)
d6af291 Log wallet transaction creation failure reasons (dexX7)
41b1b47 Remove sigops workaround for wallet transactions (dexX7)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment