forked from digibyte/digibyte
-
Notifications
You must be signed in to change notification settings - Fork 88
wallet: Fix fee estimation issue with sendmany and re-enable fallback fee #292
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
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,164 @@ | ||
| #!/usr/bin/env python3 | ||
| # Copyright (c) 2023 The DigiByte Core developers | ||
| # Distributed under the MIT software license, see the accompanying | ||
| # file COPYING or http://www.opensource.org/licenses/mit-license.php. | ||
| """Test fee estimation in the wallet with different settings.""" | ||
|
|
||
| from decimal import Decimal | ||
|
|
||
| from test_framework.blocktools import COINBASE_MATURITY_2 | ||
| from test_framework.test_framework import DigiByteTestFramework | ||
| from test_framework.util import ( | ||
| assert_equal, | ||
| assert_greater_than, | ||
| assert_raises_rpc_error, | ||
| ) | ||
|
|
||
| class WalletFeeEstimationTest(DigiByteTestFramework): | ||
| def set_test_params(self): | ||
| self.num_nodes = 2 | ||
| self.setup_clean_chain = True | ||
| self.extra_args = [ | ||
| # Node 0: Default settings (with fallback fee) | ||
| ["-fallbackfee=0.01"], | ||
| # Node 1: Fallback fee disabled | ||
| ["-fallbackfee=0"] | ||
| ] | ||
|
|
||
| def skip_test_if_missing_module(self): | ||
| self.skip_if_no_wallet() | ||
|
|
||
| def run_test(self): | ||
| # Generate blocks to get spendable outputs for all nodes | ||
| self.generate(self.nodes[0], COINBASE_MATURITY_2 + 5) | ||
| self.generate(self.nodes[1], 5) | ||
| self.sync_all() | ||
|
|
||
| # Phase 1: Initial state testing - no fee estimation history | ||
| self.log.info("Phase 1: Testing initial state with no fee estimation history") | ||
|
|
||
| # Node 0 (with fallback fee enabled) | ||
| self.log.info("Testing sendtoaddress with fallback fee enabled") | ||
| address0 = self.nodes[0].getnewaddress() | ||
| txid0 = self.nodes[0].sendtoaddress(address0, Decimal('1.0')) | ||
| tx0 = self.nodes[0].gettransaction(txid0) | ||
| self.log.info(f"Transaction fee with fallback fee enabled: {tx0['fee']}") | ||
| assert_greater_than(-tx0['fee'], 0) # Should have a non-zero fee | ||
|
|
||
| # Node 1 (fallback fee disabled) | ||
| self.log.info("Testing sendtoaddress with fallback fee disabled") | ||
| address1 = self.nodes[1].getnewaddress() | ||
| try: | ||
| txid1 = self.nodes[1].sendtoaddress(address1, Decimal('1.0')) | ||
| tx1 = self.nodes[1].gettransaction(txid1) | ||
| self.log.info(f"Transaction fee: {tx1['fee']}") | ||
| self.log.info("NOTE: Expected failure but transaction succeeded!") | ||
| except Exception as e: | ||
| self.log.info(f"Expected error: {str(e)}") | ||
| # Verify it's a fee estimation error | ||
| assert "Fee estimation failed" in str(e), "Unexpected error message" | ||
|
|
||
| # Phase 2: Build up some fee estimation data | ||
| self.log.info("Phase 2: Building fee estimation data") | ||
| self.log.info("Creating a variety of transactions to build fee estimation history") | ||
|
|
||
| # Generate transactions with varying fees | ||
| for i in range(20): | ||
| # The idea is to create transactions with different amounts to get varying fees | ||
| amount = Decimal('0.01') + (i * Decimal('0.001')) # Slightly different amounts to vary the tx size | ||
| addr = self.nodes[0].getnewaddress() | ||
| self.nodes[0].sendtoaddress(addr, amount) | ||
|
|
||
| # Create some multi-recipient transactions too | ||
| if i % 5 == 0: | ||
| recipients = {} | ||
| for j in range(3): | ||
| addr = self.nodes[0].getnewaddress() | ||
| recipients[addr] = Decimal('0.005') | ||
| self.nodes[0].sendmany("", recipients) | ||
|
|
||
| # Mine a block every few transactions | ||
| if i % 4 == 0: | ||
| self.log.info(f"Mining block to confirm transactions (iteration {i})") | ||
| self.generate(self.nodes[0], 1) | ||
| self.sync_all() | ||
|
|
||
| # Mine a final block to confirm all transactions | ||
| self.generate(self.nodes[0], 1) | ||
| self.sync_all() | ||
|
|
||
| # Phase 3: Test fee estimation after having some history | ||
| self.log.info("Phase 3: Testing fee estimation after building history") | ||
|
|
||
| # Check if node 1 (disabled fallback fee) can now estimate fees | ||
| address1 = self.nodes[1].getnewaddress() | ||
| try: | ||
| self.log.info("Testing if node with disabled fallback fee can now use fee estimation") | ||
| txid1 = self.nodes[1].sendtoaddress(address1, Decimal('0.5')) | ||
| tx1 = self.nodes[1].gettransaction(txid1) | ||
| self.log.info(f"Transaction fee with fee estimation: {tx1['fee']}") | ||
| assert_greater_than(-tx1['fee'], 0) | ||
| self.log.info("Success - fee was estimated based on history") | ||
| except Exception as e: | ||
| self.log.info(f"Fee estimation still failed: {str(e)}") | ||
| # This might happen if fee estimation doesn't have enough data yet | ||
|
|
||
| # Phase 4: Test the original issue - multiple recipients with sendmany | ||
| self.log.info("Phase 4: Testing sendmany with multiple recipients") | ||
|
|
||
| # Node 0 (with fallback fee) - should always succeed | ||
| recipients0 = {} | ||
| for i in range(30): | ||
| addr = self.nodes[0].getnewaddress() | ||
| recipients0[addr] = Decimal('0.01') | ||
|
|
||
| txid_many0 = self.nodes[0].sendmany("", recipients0) | ||
| tx_many0 = self.nodes[0].gettransaction(txid_many0) | ||
| self.log.info(f"Sendmany with 30 recipients fee (fallback enabled): {tx_many0['fee']}") | ||
|
|
||
| # Node 1 (without fallback fee) - might succeed or fail depending on fee estimation | ||
| recipients1 = {} | ||
| for i in range(30): | ||
| addr = self.nodes[1].getnewaddress() | ||
| recipients1[addr] = Decimal('0.01') | ||
|
|
||
| try: | ||
| txid_many1 = self.nodes[1].sendmany("", recipients1) | ||
| tx_many1 = self.nodes[1].gettransaction(txid_many1) | ||
| self.log.info(f"Sendmany with 30 recipients fee (no fallback): {tx_many1['fee']}") | ||
| self.log.info("Success - fee was estimated for complex transaction") | ||
| except Exception as e: | ||
| self.log.info(f"Sendmany failed: {str(e)}") | ||
| # If it still fails, it should be because of fee estimation | ||
| assert "Fee estimation failed" in str(e), "Unexpected error message" | ||
|
|
||
| # Phase 5: Test transaction chaining - the heart of the reported issue | ||
| self.log.info("Phase 5: Testing transaction chaining - the bug scenario") | ||
|
|
||
| # Node 0 (with fallback fee) should be able to create a chain | ||
| self.log.info("Creating chain of transactions with fallback fee enabled") | ||
| chain_txids = [] | ||
| for i in range(3): | ||
| chain_recipients = {} | ||
| for j in range(20): | ||
| addr = self.nodes[0].getnewaddress() | ||
| chain_recipients[addr] = Decimal('0.01') | ||
|
|
||
| txid = self.nodes[0].sendmany("", chain_recipients) | ||
| chain_txids.append(txid) | ||
| self.log.info(f"Chain transaction {i+1} sent: {txid}") | ||
|
|
||
| # Mine a block to confirm all transactions | ||
| self.generate(self.nodes[0], 1) | ||
| self.sync_all() | ||
|
|
||
| # Verify all transactions confirmed | ||
| for i, txid in enumerate(chain_txids): | ||
| tx = self.nodes[0].gettransaction(txid) | ||
| assert_equal(tx["confirmations"], 1) | ||
| self.log.info(f"Chain transaction {i+1} confirmed with fee: {tx['fee']}") | ||
|
|
||
| self.log.info("Fee estimation tests completed successfully - fallback fee solves the chaining issue") | ||
|
|
||
| if __name__ == '__main__': | ||
| WalletFeeEstimationTest().main() | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,160 @@ | ||
| #!/usr/bin/env python3 | ||
| # Copyright (c) 2023 The DigiByte Core developers | ||
| # Distributed under the MIT software license, see the accompanying | ||
| # file COPYING or http://www.opensource.org/licenses/mit-license.php. | ||
| """Test the wallet's sendmany implementation with multiple recipients that ensures no "too-long-mempool-chain" errors.""" | ||
|
|
||
| from decimal import Decimal | ||
|
||
| from test_framework.blocktools import COINBASE_MATURITY_2 | ||
| from test_framework.test_framework import DigiByteTestFramework | ||
| from test_framework.util import ( | ||
| assert_equal, | ||
| assert_greater_than, | ||
| ) | ||
|
|
||
|
|
||
| class WalletSendmanyChainTest(DigiByteTestFramework): | ||
| def set_test_params(self): | ||
| self.num_nodes = 1 | ||
| self.setup_clean_chain = True | ||
|
|
||
| def skip_test_if_missing_module(self): | ||
| self.skip_if_no_wallet() | ||
|
|
||
| def run_test(self): | ||
| # Generate blocks to get spendable outputs | ||
| self.generatetoaddress(self.nodes[0], COINBASE_MATURITY_2 + 20, self.nodes[0].getnewaddress()) | ||
|
|
||
| # Verify we have a good balance to start | ||
| initial_balance = self.nodes[0].getbalance() | ||
| self.log.info(f"Starting wallet balance: {initial_balance}") | ||
| assert_greater_than(initial_balance, 100) # Ensure we have enough funds | ||
|
|
||
| # Helper function to create multiple recipient dictionary | ||
| def create_recipients(count, amount_per_recipient): | ||
| recipients = {} | ||
| for _ in range(count): | ||
| recipients[self.nodes[0].getnewaddress()] = amount_per_recipient | ||
| return recipients | ||
|
|
||
| # Test 1: Single sendmany with multiple (5) recipients | ||
| self.log.info("Test 1: Single sendmany with 5 recipients") | ||
| recipients1 = create_recipients(5, 1) | ||
| txid1 = self.nodes[0].sendmany("", recipients1) | ||
| self.log.info(f"Transaction sent: {txid1}") | ||
| self.generate(self.nodes[0], 1) | ||
|
|
||
| # Get the transaction details and verify everything succeeded | ||
| tx1 = self.nodes[0].gettransaction(txid1) | ||
| assert_equal(tx1["confirmations"], 1) | ||
|
|
||
| # Test 2: Chain of 3 sendmany transactions without generating blocks in between | ||
| # This would have triggered the "too-long-mempool-chain" error before the fix | ||
| self.log.info("Test 2: Chain of 3 sendmany transactions without blocks") | ||
|
|
||
| # First transaction in the chain | ||
| recipients2a = create_recipients(5, 0.5) | ||
| txid2a = self.nodes[0].sendmany("", recipients2a) | ||
| self.log.info(f"First chained transaction sent: {txid2a}") | ||
|
|
||
| # Second transaction in the chain | ||
| recipients2b = create_recipients(5, 0.5) | ||
| txid2b = self.nodes[0].sendmany("", recipients2b) | ||
| self.log.info(f"Second chained transaction sent: {txid2b}") | ||
|
|
||
| # Third transaction in the chain | ||
| recipients2c = create_recipients(5, 0.5) | ||
| txid2c = self.nodes[0].sendmany("", recipients2c) | ||
| self.log.info(f"Third chained transaction sent: {txid2c}") | ||
|
|
||
| # Generate block to confirm all transactions | ||
| self.generate(self.nodes[0], 1) | ||
|
|
||
| # Verify all transactions confirmed | ||
| tx2a = self.nodes[0].gettransaction(txid2a) | ||
| tx2b = self.nodes[0].gettransaction(txid2b) | ||
| tx2c = self.nodes[0].gettransaction(txid2c) | ||
|
|
||
| assert_equal(tx2a["confirmations"], 1) | ||
| assert_equal(tx2b["confirmations"], 1) | ||
| assert_equal(tx2c["confirmations"], 1) | ||
|
|
||
| # Test 3: Chain of 5 sendmany with many recipients each (10) | ||
| self.log.info("Test 3: Chain of 5 sendmany with 10 recipients each") | ||
|
|
||
| txids = [] | ||
| for i in range(5): | ||
| recipients = create_recipients(10, 0.2) | ||
| txid = self.nodes[0].sendmany("", recipients) | ||
| self.log.info(f"Chain transaction {i+1} sent: {txid}") | ||
| txids.append(txid) | ||
|
|
||
| # Generate block to confirm all transactions | ||
| self.generate(self.nodes[0], 1) | ||
|
|
||
| # Verify all transactions confirmed | ||
| for i, txid in enumerate(txids): | ||
| tx = self.nodes[0].gettransaction(txid) | ||
| assert_equal(tx["confirmations"], 1) | ||
|
|
||
| # Test 4: Extreme case - Single sendmany with 30 recipients | ||
| # This specifically tests the case reported in the issue | ||
| self.log.info("Test 4: Extreme case - Single sendmany with 30 recipients") | ||
| recipients_extreme = create_recipients(30, 0.1) | ||
| txid_extreme = self.nodes[0].sendmany("", recipients_extreme) | ||
| self.log.info(f"Extreme transaction sent: {txid_extreme}") | ||
| self.generate(self.nodes[0], 1) | ||
|
|
||
| # Verify transaction confirmed | ||
| tx_extreme = self.nodes[0].gettransaction(txid_extreme) | ||
| assert_equal(tx_extreme["confirmations"], 1) | ||
|
|
||
| # Test 5: Multiple chained extreme sendmany operations (3 chained sendmany with 30 recipients each) | ||
| # This is a stress test combining both the chaining and large recipient count | ||
| self.log.info("Test 5: Multiple chained extreme sendmany operations (3x30 recipients)") | ||
|
|
||
| extreme_txids = [] | ||
| for i in range(3): | ||
| recipients_extreme_chain = create_recipients(30, 0.05) | ||
| txid = self.nodes[0].sendmany("", recipients_extreme_chain) | ||
| self.log.info(f"Extreme chain transaction {i+1} sent: {txid}") | ||
| extreme_txids.append(txid) | ||
|
|
||
| # Generate block to confirm all transactions | ||
| self.generate(self.nodes[0], 1) | ||
|
|
||
| # Verify all transactions confirmed | ||
| for i, txid in enumerate(extreme_txids): | ||
| tx = self.nodes[0].gettransaction(txid) | ||
| assert_equal(tx["confirmations"], 1) | ||
|
|
||
| # Test 6: Long chain of transactions with 20 recipients each | ||
| # This tests a longer chain than previous tests - creates a sequence of | ||
| # 7 consecutive transactions without generating a block | ||
| self.log.info("Test 6: Long chain of transactions (7x20 recipients)") | ||
|
|
||
| long_chain_txids = [] | ||
| for i in range(7): | ||
| recipients_long_chain = create_recipients(20, 0.01) | ||
| txid = self.nodes[0].sendmany("", recipients_long_chain) | ||
| self.log.info(f"Long chain transaction {i+1} sent: {txid}") | ||
| long_chain_txids.append(txid) | ||
|
|
||
| # Generate block to confirm all transactions | ||
| self.generate(self.nodes[0], 1) | ||
|
|
||
| # Verify all transactions confirmed | ||
| for i, txid in enumerate(long_chain_txids): | ||
| tx = self.nodes[0].gettransaction(txid) | ||
| assert_equal(tx["confirmations"], 1) | ||
|
|
||
| # Verify we have a non-zero balance left | ||
| final_balance = self.nodes[0].getbalance() | ||
| self.log.info(f"Final wallet balance: {final_balance}") | ||
| assert_greater_than(final_balance, 0) | ||
|
|
||
| self.log.info("All sendmany chain tests completed successfully") | ||
|
|
||
|
|
||
| if __name__ == '__main__': | ||
| WalletSendmanyChainTest().main() | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.