bcndev / bytecoin

GNU Lesser General Public License v3.0
232 stars 221 forks source link

Transaction stuck in the mempool #80

Open ghost opened 6 years ago

ghost commented 6 years ago

I’m reading the code to understand it then I have created my own cryptocurrency and everything works fine but sometimes some transactions are stuck in mempool. The transaction is sent ok by the sender and it is received by the receiver but the transaction amount is locked or unconfirmed for ever. I’ve try to figure out this but I don’t understand what is happening. I write here because I don’t know if this is a bug or this is normal in a new cryptoccurrency with low transactions, in this last case why is not the transaction blocked in the sender side reporting an error?

This happen to me in the last commit release v3.2.1 commits (prime) and v3.2.2 now

bcndev commented 6 years ago

@nilhcraiv It's hard to say what's going on in your case. Do you have a stable reproducer of the problem?

ghost commented 6 years ago

Detailed steps to reproduce the problem:

  1. Build the new Cryptourrency

    First we build the new cryptocurrency as follows:

    bytecoin/src/Core/Currency.cpp

    //This an example GENESIS BLOCK, you can use other if you want
    std::string genesis_coinbase_tx_hex = "010a01ff0001bbe1abb42c029b2e4c0281c0b02e7c53291a94d1d0cbff8883f8024f5142ee494ffbbd0880712101e276fe77a8511f282da163c9b07129676335f6c00786a68a47f887a1db961291";
    
    //You can also change next_effective_difficulty() (in order to reduce the time for this test)
    Difficulty Currency::next_effective_difficulty(uint8_t block_major_version, std::vector<Timestamp> timestamps,
    std::vector<CumulativeDifficulty> cumulative_difficulties) const {
    return 10;//For example
    }

    bytecoin/src/CryptoNoteConfig.hpp

    //Change these lines
    
    const uint64_t CRYPTONOTE_PUBLIC_ADDRESS_BASE58_PREFIX = 143;
    ...
    const uint64_t MONEY_SUPPLY          = UINT64_C(25000000000000000);
    const unsigned EMISSION_SPEED_FACTOR = 21;
    ...
    const size_t CRYPTONOTE_BLOCK_GRANTED_FULL_REWARD_ZONE = 100000;  // size of block (bytes) after which reward for block calculated using block size
    ...
    const uint64_t MINIMUM_DIFFICULTY_FROM_V2 = 10;  // TODO - complete fix in the next hardfork
    ...
    const uint32_t UPGRADE_HEIGHT_V2 = 1;
    const uint32_t UPGRADE_HEIGHT_V3 = 2;
    ...
    const char CRYPTONOTE_NAME[] = "new_coin";
    ...
    const int P2P_DEFAULT_PORT        = 48080;
    const int RPC_DEFAULT_PORT        = 48081;
    const int WALLET_RPC_DEFAULT_PORT = 48082;
    ...
    const char P2P_STAT_TRUSTED_PUBLIC_KEY[] = "";
    ...
    const char *const CHECKPOINT_PUBLIC_KEYS[] = {};
    const char *const CHECKPOINT_PUBLIC_KEYS_TESTNET[] = {};
    const char *const SEED_NODES[] = {
            "127.0.0.1:58080", "127.0.0.1:48080"};
    struct CheckpointData {
            uint32_t height;
            const char *hash;
    };
    //If you use this GENESIS BLOCK this is your CHECKPOINTS at 0-> 010a01ff0001bbe1abb42c029b2e4c0281c0b02e7c53291a94d1d0cbff8883f8024f5142ee494ffbbd0880712101e276fe77a8511f282da163c9b07129676335f6c00786a68a47f887a1db961291
                constexpr const CheckpointData CHECKPOINTS[] = { {0, "fe2634add0aa1c8a1f800c8cdcc2394999520d320705de1aaa384c978282d08d"} };
                }

    bytecoin/src/Core/BlockChain.hpp

    //change 7 -> 0 we have not CHECKPOINT_PUBLIC_KEYS in CryptoNoteConfig.hpp.
    typedef std::array<Height, 0> CheckPointDifficulty;  // size must be == m_currency.get_checkpoint_keys_count()

    bytecoin/src/Core/Config.cpp

    //change bytecoin network for example:
    const static UUID BYTECOIN_NETWORK{{0x11, 0x41, 0x1c, 0x20, 0x44, 0x44, 0xaa, 0xa3, 0xaa, 0x35, 0x63, 0x56, 0x88, 0xac, 0x12, 0x68}};
  2. Build it and run at least two daemons with two wallets to do this test.

  3. Mine 720 blocks using only one address. In my case I used -> QvKsfGFCbAMAhY3NjcJDo3j7Yy4AaYYMUd2HgPLV48kJEBjsP29cNT53b4ApZ6BcJRFgEmYqVEJBz5MJyRNPnW6z1eb3nweBe

  4. Balance

    With 720 resolved blocks this is the balance returned by "get_balance" method:

    curl -s -u user:password -X POST http://127.0.0.1:58082/json_rpc -H 'Content-Type: application/json-rpc' -d '{
    "jsonrpc": "2.0",
    "id": "0",
    "method": "get_balance",
    "params": { }
    }'
    "locked_or_unconfirmed":190670072768,"locked_or_unconfirmed_outputs":112,
    "spendable":8390572000000,"spendable_dust":351515350,
    "spendable_dust_outputs":1407,"spendable_outputs":3357
  5. Send 8000000000000

    In this point, if you try to send 8000000000000 (8 decimals then this is 80000 coins) the wallet shows you TRANSACTION DOES NOT FIT IN BLOCK.

    bytecoin/src/rpc_api.hpp

    TRANSACTION_DOES_NOT_FIT_IN_BLOCK = -302,  // Sender will have to split funds into several transactions
  6. Send 4000000000000 instead of 8000000000000

    Example destination address -> QvKmESVvfoUB9Gws88L5WFau3P46ZM2uB7WrUwWiSo3xEGVizy298QDUhofi5gzjDsf12SUvsQmhXSBUpqox4eEY29LZW7SoP

    Example source address --> QvKsfGFCbAMAhY3NjcJDo3j7Yy4AaYYMUd2HgPLV48kJEBjsP29cNT53b4ApZ6BcJRFgEmYqVEJBz5MJyRNPnW6z1eb3nweBe

    curl -s -u user:password -X POST http://127.0.0.1:48082/json_rpc -H 'Content-Type: application/json-rpc' -d '{
        "jsonrpc": "2.0",
        "id": "0",
        "method": "create_transaction",
            "params": {
              "transaction": {
                "anonymity": 0,
                "payment_id": "",
                "transfers": [
                  {
                    "address": "QvKmESVvfoUB9Gws88L5WFau3P46ZM2uB7WrUwWiSo3xEGVizy298QDUhofi5gzjDsf12SUvsQmhXSBUpqox4eEY29LZW7SoP",
                      "amount": 4000000000000
                   }
                 ]
               },
               "spend_addresses": [
                 "QvKsfGFCbAMAhY3NjcJDo3j7Yy4AaYYMUd2HgPLV48kJEBjsP29cNT53b4ApZ6BcJRFgEmYqVEJBz5MJyRNPnW6z1eb3nweBe"
               ],
               "change_address": "QvKsfGFCbAMAhY3NjcJDo3j7Yy4AaYYMUd2HgPLV48kJEBjsP29cNT53b4ApZ6BcJRFgEmYqVEJBz5MJyRNPnW6z1eb3nweBe",
            "unlock_time": 0
            }
    }'

    The binary_transaction returned is very big, it has 84672 characters.

    Send the binary_transaction:

    curl -s -u use:password -X POST http://127.0.0.1:48082/json_rpc -H 'Content-Type: application/json-rpc' -d '{
        "jsonrpc": "2.0",
        "id": "0",
        "method": "send_transaction",
        "params": {
            "binary_transaction": ""//The binary transaction returned by create_transaction method before.
            }
    }'

    HERE IS THE ISSUE This transaction is sent correctly and it is received but the transaction amount is locked or unconfirmed for ever in the destination balance.

  7. Transaction blocked.

    Now you can mine 20, 40, hundreds, thousands, etc. blocks but this transaction will never be confirmed. It will remain blocked in the destination account.

SEARCHING FOR BUGS

In bytecoin/src/Core/BlockChainState.cpp functions "BlockChainState::create_mining_block_template2" and "BlockChainState::create_mining_block_template" never include this transaction because it is too big.

This happens here, tx_size is bigger than block_size_limit then the transaction will never be included.

//bytecoin/src/Core/BlockChainState.cpp
if (txs_size + tx_size > block_size_limit)
    continue;

block_size_limit is calculated as follows:

auto next_block_granted_full_reward_zone = m_currency.block_granted_full_reward_zone_by_block_version(b->major_version);//In this case is CRYPTONOTE_BLOCK_GRANTED_FULL_REWARD_ZONE = 100000
auto effective_size_median = std::max(m_next_median_size, next_block_granted_full_reward_zone);

auto max_total_size = (125 * effective_size_median) / 100;
auto max_cumulative_size = m_currency.max_block_cumulative_size(*height);
max_total_size = std::min(max_total_size, max_cumulative_size) - m_currency.miner_tx_blob_reserved_size;
const size_t block_size_limit = max_total_size;

m_currency.max_block_cumulative_size() depends of current height as follows:

uint32_t Currency::max_block_cumulative_size(Height height) const {
    assert(height <= std::numeric_limits<uint64_t>::max() / max_block_size_growth_speed_numerator);
    uint64_t max_size = static_cast<uint64_t>(
        max_block_size_initial +
        (height * max_block_size_growth_speed_numerator) / max_block_size_growth_speed_denominator);
    assert(max_size >= max_block_size_initial);
    return static_cast<uint32_t>(max_size);
}

In our new currency we are at the beginning therefore max_size is small, but will increase in the future. This will block the transaction until max_size will be greater but the problem is that this will happen in a very long time... Also, it is limited by CRYPTONOTE_BLOCK_GRANTED_FULL_REWARD_ZONE

QUESTIONS

When I tried to send 80k the wallet showed TRANSACTION_DOES_NOT_FIT_IN_BLOCK but when I sent 40k the wallet allows me to do it, why? Is it possible to calculate the size of the transaction before send it? How can these transactions be unlocked? If it is not possible, they must be returned.

RELATED ISSUES

https://github.com/forknote/cryptonote-generator/issues/54 https://github.com/turtlecoin/meta/issues/21

It seems that Monero solves this with rescan_spent method.

*EDITED

I fixed temporary this issue. I change this condition in TransactionBuilder.cpp as follows.

bytecoin/src/Core/TransactionBuilder.cpp

if (tx_size > effective_median_size)
            return "TRANSACTION_DOES_NOT_FIT_IN_BLOCK";

I have changed it for this code (The same condition as BlockChainState::create_mining_block_templateX)

unsigned long max_total_size      = (125 * effective_median_size) / 100;
unsigned long max_cumulative_size = static_cast<unsigned long> (m_currency.max_block_cumulative_size(block_height));
max_total_size           = std::min(max_total_size, max_cumulative_size) - m_currency.miner_tx_blob_reserved_size;
const size_t block_size_limit = max_total_size;

if (tx_size > block_size_limit)
{
  m_log(logging::ERROR) << "Transaction does not fit in block. tx_size: "  << tx_size
                        << " block_size_limit: " << block_size_limit
                        << " fee_per_byte: " << fee_per_byte
                        << " total_amount: " << total_amount
                        << " fee: " << fee
                        << " anonymity: " << anonymity
                        << " effective_median_size: " << effective_median_size
                        << " max_total_size = (125 * effective_median_size) / 100: " << max_total_size << std::endl;
  return "TRANSACTION_DOES_NOT_FIT_IN_BLOCK";
}

Now, instead of sending a transaction with a big size the wallet return TRANSACTION_DOES_NOT_FIT_IN_BLOCK in order to avoid unconfirmed transactions.

I think both conditions should be the same in both files, right? (TransactionBuilder.cpp and BlockChainState.cpp)

On the other hand, how can I return the transactions that remain unconfirmed? I think this could be happening in the bytecoin network at the moment. A method to unlock this type of transaction should be implemented.

4egod commented 6 years ago

Look at BlockChain State::add transaction method. There is a piece of code: // Timestamp g_timestamp = read_first_seen_timestamp(tid); // if (g_timestamp != 0 && now > g_timestamp + m_config.mempool_tx_live_time) // return AddTransactionResult::TOO_OLD; You can uncomment it and then your bad transactions will remove from the mempool. It's not the best solution, but it works