From d2c01fbe14b8adbfd4af2e110a58a772c37eecb8 Mon Sep 17 00:00:00 2001 From: Peter Bushnell Date: Sun, 13 Dec 2020 11:39:29 +0000 Subject: [PATCH] Calculate fee for Omni TXs dynamically --- src/omnicore/wallettxbuilder.cpp | 92 ++++++++++++++++++------- src/omnicore/walletutils.cpp | 12 +--- src/omnicore/walletutils.h | 2 +- test/functional/omni_calculate_fee.py | 99 +++++++++++++++++++++++++++ test/functional/test_runner.py | 1 + 5 files changed, 171 insertions(+), 35 deletions(-) create mode 100644 test/functional/omni_calculate_fee.py diff --git a/src/omnicore/wallettxbuilder.cpp b/src/omnicore/wallettxbuilder.cpp index 6b30e63cdd7b9..08b5e52d23f09 100644 --- a/src/omnicore/wallettxbuilder.cpp +++ b/src/omnicore/wallettxbuilder.cpp @@ -91,13 +91,7 @@ int WalletTxBuilder( vecSend.push_back(std::make_pair(scriptPubKey, 0 < referenceAmount ? referenceAmount : OmniGetDustThreshold(scriptPubKey))); } - // Select the inputs - if (0 > mastercore::SelectCoins(*iWallet, senderAddress, coinControl, outputAmount)) { return MP_INPUTS_INVALID; } - - // Now we have what we need to pass to the wallet to create the transaction, perform some checks first - - if (!coinControl.HasSelected()) return MP_ERR_INPUTSELECT_FAIL; - + // Create CRecipients for outputs std::vector vecRecipients; for (size_t i = 0; i < vecSend.size(); ++i) { const std::pair& vec = vecSend[i]; @@ -105,11 +99,40 @@ int WalletTxBuilder( vecRecipients.push_back(recipient); } - // Ask the wallet to create the transaction (note mining fee determined by Bitcoin Core params) - CAmount nFeeRet = 0; - int nChangePosInOut = -1; + CAmount nFeeRequired{std::max(minFee, iWallet->getMinimumFee(1000, coinControl, nullptr, nullptr))}; + CTransactionRef wtxNew; std::string strFailReason; - auto wtxNew = iWallet->createTransaction(vecRecipients, coinControl, true /* sign */, nChangePosInOut, nFeeRet, strFailReason, false, minFee); + CAmount nFeeRet{0}; + bool createTX{true}; + + while (createTX) { + // Select the inputs + auto selected = mastercore::SelectCoins(*iWallet, senderAddress, coinControl, outputAmount + nFeeRequired); + + // Did not select anything at all! + if (!coinControl.HasSelected()) { + return MP_ERR_INPUTSELECT_FAIL; + } + + // Could not select to enough to cover outputs and fee + if (selected < outputAmount) { + return MP_INPUTS_INVALID; + } + + // Ask the wallet to create the transaction (note mining fee determined by Bitcoin Core params) + int nChangePosInOut = -1; + wtxNew = iWallet->createTransaction(vecRecipients, coinControl, true /* sign */, nChangePosInOut, nFeeRet, strFailReason, false, minFee); + + // TX creation was a success or fee no longer incremeneintg + if (wtxNew || nFeeRet <= nFeeRequired) { + createTX = false; + } else { + // Set new fee required + nFeeRequired = nFeeRet; + + PrintToLog("Increasing fee. nFeeRequired: %d selected: %d outputAmount: %d\n", nFeeRequired, selected, outputAmount); + } + } if (!wtxNew) { PrintToLog("%s: ERROR: wallet transaction creation failed: %s\n", __func__, strFailReason); @@ -391,26 +414,45 @@ int CreateDExTransaction(interfaces::Wallet* pwallet, const std::string& buyerAd // Calculate dust for Exodus output CAmount dust = OmniGetDustThreshold(exodus); - // Select the inputs required to cover amount, dust and fees - if (0 > mastercore::SelectCoins(*pwallet, buyerAddress, coinControl, nAmount + dust)) { - return MP_INPUTS_INVALID; - } - - // Make sure that we have inputs selected. - if (!coinControl.HasSelected()) { - return MP_ERR_INPUTSELECT_FAIL; - } - // Create CRecipients for outputs std::vector vecRecipients; vecRecipients.push_back({exodus, dust, false}); // Exodus vecRecipients.push_back({destScript, nAmount, false}); // Seller - // Ask the wallet to create the transaction (note mining fee determined by Bitcoin Core params) - CAmount nFeeRet = 0; - int nChangePosInOut = -1; + CAmount nFeeRequired{pwallet->getMinimumFee(1000, coinControl, nullptr, nullptr)}; + CTransactionRef wtxNew; std::string strFailReason; - auto wtxNew = pwallet->createTransaction(vecRecipients, coinControl, true /* sign */, nChangePosInOut, nFeeRet, strFailReason, false); + CAmount nFeeRet{0}; + CAmount outputAmount{nAmount + dust}; + bool createTX{true}; + + while (createTX) { + // Select the inputs + auto selected = mastercore::SelectCoins(*pwallet, buyerAddress, coinControl, outputAmount + nFeeRequired); + + // Did not select anything at all! + if (!coinControl.HasSelected()) { + return MP_ERR_INPUTSELECT_FAIL; + } + + // Could not select to enough to cover outputs and fee + if (selected < outputAmount) { + return MP_INPUTS_INVALID; + } + + int nChangePosInOut = -1; + wtxNew = pwallet->createTransaction(vecRecipients, coinControl, true /* sign */, nChangePosInOut, nFeeRet, strFailReason, false); + + // TX creation was a success or fee no longer incremeneintg + if (wtxNew || nFeeRet <= nFeeRequired) { + createTX = false; + } else { + // Set new fee required + nFeeRequired = nFeeRet; + + PrintToLog("Increasing fee. nFeeRequired: %d selected: %d outputAmount: %d\n", nFeeRequired, selected, outputAmount); + } + } if (!wtxNew) { return MP_ERR_CREATE_TX; diff --git a/src/omnicore/walletutils.cpp b/src/omnicore/walletutils.cpp index 8011181b669e5..7ff6868ff4515 100644 --- a/src/omnicore/walletutils.cpp +++ b/src/omnicore/walletutils.cpp @@ -202,18 +202,12 @@ int64_t GetEconomicThreshold(interfaces::Wallet& iWallet, const CTxOut& txOut) } #ifdef ENABLE_WALLET -int64_t SelectCoins(interfaces::Wallet& iWallet, const std::string& fromAddress, CCoinControl& coinControl, int64_t additional) +int64_t SelectCoins(interfaces::Wallet& iWallet, const std::string& fromAddress, CCoinControl& coinControl, int64_t amountRequired) { // total output funds collected int64_t nTotal = 0; int nHeight = ::ChainActive().Height(); - // select coins to cover up to 3 kB max. transaction size - CAmount nMax = 3 * GetEstimatedFeePerKb(iWallet); - - // if referenceamount is set it is needed to be accounted for here too - if (0 < additional) nMax += additional; - std::map tx_status; const std::vector& transactions = iWallet.getWalletTxsDetails(tx_status); @@ -271,11 +265,11 @@ int64_t SelectCoins(interfaces::Wallet& iWallet, const std::string& fromAddress, nTotal += txOut.nValue; - if (nMax <= nTotal) break; + if (amountRequired <= nTotal) break; } } - if (nMax <= nTotal) break; + if (amountRequired <= nTotal) break; } return nTotal; diff --git a/src/omnicore/walletutils.h b/src/omnicore/walletutils.h index 0e4fbac675479..316d0b5a119a5 100644 --- a/src/omnicore/walletutils.h +++ b/src/omnicore/walletutils.h @@ -41,7 +41,7 @@ int64_t GetEconomicThreshold(interfaces::Wallet& iWallet, const CTxOut& txOut); #ifdef ENABLE_WALLET /** Selects spendable outputs to create a transaction. */ -int64_t SelectCoins(interfaces::Wallet& iWallet, const std::string& fromAddress, CCoinControl& coinControl, int64_t additional = 0); +int64_t SelectCoins(interfaces::Wallet& iWallet, const std::string& fromAddress, CCoinControl& coinControl, int64_t amountRequired = 0); /** Selects all spendable outputs to create a transaction. */ int64_t SelectAllCoins(interfaces::Wallet& iWallet, const std::string& fromAddress, CCoinControl& coinControl); diff --git a/test/functional/omni_calculate_fee.py b/test/functional/omni_calculate_fee.py new file mode 100644 index 0000000000000..c5578d535a9b2 --- /dev/null +++ b/test/functional/omni_calculate_fee.py @@ -0,0 +1,99 @@ +#!/usr/bin/env python3 +# Copyright (c) 2017-2018 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test Omni fee calculation.""" + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal + +class OmniFeeCalculation(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 2 + self.setup_clean_chain = True + self.extra_args = [[], ["-omnidebug=all", "-paytxfee=0.00003"]] # 3,000 Sats per KB + + def run_test(self): + self.log.info("test fee calculation") + + node0 = self.nodes[0] + node1 = self.nodes[1] + + # Preparing some mature Bitcoins + coinbase_address = node0.getnewaddress() + node0.generatetoaddress(130, coinbase_address) + + # Obtaining a master address to work with + address = node1.getnewaddress() + + # Send lots of small dust inputs to create a large TX + for _ in range(3): + for _ in range(50): + node0.sendtoaddress(address, 0.00002000) + node0.generatetoaddress(10, coinbase_address) + + # Test small transaction with single input + txid = node1.omni_sendissuancefixed(address, 1, 2, 0, "", "", "TST", "", "", "1000") + node1.generatetoaddress(2, coinbase_address) + + # Checking the transaction was valid... + result = node1.omni_gettransaction(txid) + print(result) + assert_equal(result['valid'], True) + + # Minimum fee is 1KB and 3Sats/byte rate vins to cover 3,000 Sats will be selected. + assert_equal(len(node1.getrawtransaction(txid, 1)['vin']), 2) + + # Large data + large_data = "111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111" + + # Create large transaction, will fail if fee requirement not met + txid = node1.omni_sendissuancecrowdsale(address, 1, 2, 0, large_data, large_data, large_data, large_data, large_data, 1, "1", 32533598549, 0, 0) + node1.generatetoaddress(1, coinbase_address) + + # Checking the transaction was valid... + result = node1.omni_gettransaction(txid) + assert_equal(result['valid'], True) + + # Test DEx calls + + # Create and fund new address + omni_address = node0.getnewaddress() + node0.sendtoaddress(omni_address, 20) + node0.generatetoaddress(1, coinbase_address) + + # Participating in the Exodus crowdsale to obtain some OMNI + txid = node0.sendmany("", {"moneyqMan7uh8FqdCA2BV5yZ8qVrc9ikLP": 10, omni_address: 4}) + node0.generatetoaddress(10, coinbase_address) + + # Create Offer + txid = node0.omni_senddexsell(omni_address, 1, "1", "1", 10, "0.0001", 1) + node0.generatetoaddress(1, coinbase_address) + + # Checking the transaction was valid... + result = node0.omni_gettransaction(txid) + assert_equal(result['valid'], True) + + # Accept the DEx offer + txid = node1.omni_senddexaccept(address, omni_address, 1, "1") + node1.generatetoaddress(1, coinbase_address) + + # Checking the transaction was valid... + result = node1.omni_gettransaction(txid) + print(result) + assert_equal(result['valid'], True) + + # Give buyer bitcoin to pay with, only fee left to pay + node0.sendtoaddress(address, 1) + node0.generatetoaddress(1, coinbase_address) + + # Pay for the DEx offer, tests min fee arg to WalletTxBuilder + txid = node1.omni_senddexpay(address, omni_address, 1, "1") + node1.generatetoaddress(1, coinbase_address) + + # Checking the transaction was valid... + result = node1.omni_gettransaction(txid) + assert_equal(result['purchases'][0]['valid'], True) + +if __name__ == '__main__': + OmniFeeCalculation().main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 2e9a03b2c92c5..988f516c81777 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -224,6 +224,7 @@ 'feature_help.py', 'feature_shutdown.py', 'framework_test_script.py', + 'omni_calculate_fee.py', 'omni_reorg.py', 'omni_clientexpiry.py', 'omni_metadexprices.py',