XBridge Invalid amount bug

[shrnkld]

There is an intermittent bug with xbridge order posting that results in an 'invalid amount' error (see screenshot attached)

We need to find the route of this issue but the logs don't provide enough info to track it down. Currently we don't have the exact steps to reproduce but the following process can result in the error:

Steps to reproduce:

1. start and unlock blocknet core wallet2. start and unlock LTC local wallet3. start block DX4. place a partial order via console or cli calling dxmakepartialorder) for eg: dxMakePartialOrder LTC 0.010853 LNu2ja6eLrttWj4VwpABFAB4dBVeJBjSAB BLOCK 1 BVADycgu66et8PANEwLNY1uCHbBZFuC8Wp 0.001085 true

Result: error 1026 - invalid amount

Important note: this error is intermittent as you can see from the screenshots attached. Sometimes the same order works and others it fails.

To do:

• [ ] investigate what causes xbridge to return the ‘invalid amount' error when dxmakepartialorder is called
• [ ] add further logging to xbridge if this cannot be determined from the current code:
• [ ] each UTXOs details should be available in the log

the maximum number of utxos on the order was exceeded (UTXOs empty):

if (utxos.empty()) {...
    xbridge::LogOrderMsg(log_obj, "failed to create order, the maximum number of utxos on the order was exceeded", __FUNCTION__);
                    return xbridge::Error::UTXOS_EMPTY;
                }

insufficient funds on partial order:

xbridge::LogOrderMsg(log_obj, "failed to create order, insufficient funds on partial order", __FUNCTION__);
                    return xbridge::Error::INSUFFICIENT_FUNDS_PARTIAL_ORDER;

the maximum number of utxos on the order was exceeded:

if (ptr->usedCoins.size() > xBridgePartialOrderMaxUtxos) {...
  xbridge::LogOrderMsg(log_obj, "failed to create order, the maximum number of utxos on the order was exceeded", __FUNCTION__);
                    return xbridge::Error::EXCEEDED_MAX_UTXOS;

• [ ] add the below new errors in blocknet/src/xbridge/util/xbridgeerror.cpp

case Error::UTXOS_EMPTY:
return "Failed to create order, the maximum number of utxos on the order was exceeded " + argument;

case Error::INSUFFICIENT_FUNDS_PARTIAL_ORDER:
return "failed to create order, insufficient funds on partial order " + argument;

case Error::EXCEEDED_MAX_UTXOS:
return "failed to create order, the maximum number of utxos on the order was exceeded " + argument;

[shrnkld]

Info for testing:

• dxMakePartialOrder with use_all_funds=false, auto_split=true
• The bug largly depends on the size and count of utxo in a wallet/address, hence it is important to be very selective about which utxo can be used (use_all_funds=false)

Creates an order with no remainder: (5 x 0.02):

dxMakePartialOrder "SYS" "0.1" "SYSCOIN_ADDRESS" "BLOCK" "0.01" "BLOCK_ADDRESS" "0.02" true false true

Creates an order with a remainder: (5 x 0.02 + 0.005):

dxMakePartialOrder "SYS" "0.105" "SYSCOIN_ADDRESS" "BLOCK" "0.01" "BLOCK_ADDRESS" "0.02" true false true

0.02 - split utxo (Note: the resulting utxo seen in the wallet will be slightly larger, because it also accounts for the tx fee, e.g 0.0204 in the case of SYS) 0.005 - remainder

Some test cases: (create orders from a set of utxo with the follow properties)

• split utxo fully match (no remainder, with any remainder, with matching remainder) - meaning all split utxo have correct size & fee
• split utxo partially match, (no remainder, with any remainder, with matching remainder) - meaning some split utxo have correct size & fee (not all of them)
• any remainder being: 1-3 large ones, lots of small ones
• lots of very small inputs (minimum_size tiny bit larger than utxo size, minimum_size tiny bit smaller than utxo size) - use dxSplitInputs
• fiddle around, try to get very close to amount on address == amount needed to create order (maker_size + any kind of fee)

[shrnkld]

Test builds: https://gitlab.com/blocknetdx/blocknet/-/pipelines/341989858

[shrnkld]

Latest test builds: https://gitlab.com/blocknetdx/blocknet/-/pipelines/345361961

[shrnkld]

It is essentially caused by the CAmount to double conversion, which can lead to slight rounding errors. During the order creation process, the client will sign a message (which also includes the amount) to prove ownership. This is being signed:

std::string UtxoEntry::toString() const
{
    std::ostringstream o;
    o << txId << ":" << vout << ":" << amount << ":" << address;
    return o.str();
}

https://github.com/blocknetdx/blocknet/blob/master/src/xbridge/xbridgewalletconnector.cpp#L28 (note the usage of type double for amount)

Now when the exchange (snode) tries to verify the signed message, it will come up with a slightly different amount and therefore different message. Consequently, the exchange believes the signature is invalid. That is what caused the problem.

Solution: - Long-term: Get rid of the double, replace it with CAmount, which is essentially an integer (this might happen anyway with the 18 decimal thing?) - there is a reason bitcoin is doing the math in int - Short-term: Reload amount values before signing in a similar fashion as the exchange does (both client and exchange should end up with the same amount)

New builds: https://gitlab.com/blocknetdx/blocknet/-/pipelines/345361961

The PR now contains fixes for 5 things:

• implicit conversion, were comparing double to CAmount https://github.com/rikublock/blocknet/commit/fc9d7172890ddcc6b99335852bb2b384c5bad9f4
• two fee calculation problems (inaccurate) https://github.com/rikublock/blocknet/commit/ac013fb1cd952ce815b044644f5978830d214014
• exactly 10 outputs + remainder problem https://github.com/rikublock/blocknet/commit/0b4511f5221c3afb7e98263b6709287409d7d503
• amount discrepancies client/snode https://github.com/rikublock/blocknet/commit/e2a2b887d734de59d96c10080608c3cbc3bb259d

[shrnkld]

Invalid amount Issue still persists when using a combination of Block core and XLite for other assets due to the fix/patch use of gettxout to update the amounts.

In most cases the utxo being updated will still have 0 confirmations because we just created the transaction. This isn't really an issue as long as the core wallet is run locally, the wallet will still recognise it as a "wallet transaction" and as such return the information. This doesn't appear to be the case when using xlite.