Fix : Reject invalid instantsend transaction

[Alex-Werner]

Using sendrawtransaction with arguments asking for a instantsend transaction (third argument instantsend at true).
If our hexstring use a UTXO having less than 6 confirmations, it will be invalid.
While we will have our response being Not a valid InstantSend transaction, see debug.log for more info, the sent rawtx, will still be processed as a normal transaction.

The problem is that :

• The requester won't receive the txid from RPC and therefore might not be aware that his transaction has been broadcasted (but in standard tx instead of instantsend tx).
• The requested might want to use another input with enough confirmation instead and therefore do not want to send a standard transaction.
• The daemon had chosen for the requester to broadcast his transaction after all which should be decide by the requester.
• We have returned an error but still broadcast anyway.

This is due because we first add to the mempool the transaction, and only after, we check that it's indeed a instantSend valid tx. If it's not we throw an error.

See exemple here :
http://dev-test.insight.dashevo.org/insight-api-dash/tx/bf399bcb7df27849590334bf660d79ac414a9a4f955a9c596d8dfc1ad92a851b

• Fee is 0.001 and input number is 1 = Criteria in term of number of input (<100) and fee (0.001 * COIN) is met.
• Difference between confirmation of input and confirmation of tx is 5, meaning that the utxo didn't comply with the criteria of 6 conf minimum.

But you still see that the transaction has been sent, with txlock:false.

How to reproduce :

• Create new address and fund with tDash.
• When conf of the funding tx is between 1 and 6 (excluded) : prepare rawtx and instantSend them.

Fix :
By moving the verification and error throwing before we add to the mempool, the transaction won't be propagated at all.

Tested working on a Ubuntu 17.04 through Insight-API v0.5 /tx/sendix.
I didn't found any test-case to update.

[codablock]

Just curious: Does ProcessTxLockRequest do all the required verification that AcceptToMemoryPool is doing? Or does ProcessTxLockRequest rely on the caller having this already done? In case it relies on the caller, I'm not sure what would happen if for example a too young coinbase is used? As I understand it, it would do the lock request and then fail to add it to the mempool. Wouldn't this result in the coins being locked and unspendable afterwards (at least as long as the lock is active)?

[UdjinM6]

ProcessTxLockRequest doesn't "lock" anything actually. The main purpose of it is to create so called Lock Candidate to hold votes and "lock" later. But since request is invalid it won't be broadcasted, no MN will vote, so Lock Candidate will just expire and will be deleted. It's ok to have coins in many competing Lock Candidates until one of them is "locked" (i.e. has enough MN votes).

[UdjinM6]

utACK

[gladcow]

I have tested it on testnet, InstantSend is working, sendrawtransaction sends instantsend if input is correct and sends nothing if we haven't enough confirmations. ACK