fix(electrum): Fix fetch_prev_txout

[ValuedMammal]

Previously we inserted every TxOut of a previous tx at the same outpoint, which is incorrect because an outpoint only points to a single TxOut. Now just get the TxOut corresponding to the txin prevout and insert it with its outpoint.

Notes to the reviewers

The bug in question was demonstrated in a discord comment https://discord.com/channels/753336465005608961/1239693193159639073/1239704153400414298 but I don't think we've opened an issue yet. Essentially, because of a mismatch between the outpoint and txout stored in TxGraph, we weren't summing the inputs correctly which caused calculate_fee to fail with NegativeFee error.

fixes #1419

Checklists

All Submissions:

• I've signed all my commits
• I followed the contribution guidelines
• I ran cargo fmt and cargo clippy before committing

Bugfixes:

• I've added tests to reproduce the issue which are now passing

[notmandatory]

This looks like the right solution but Is it possible to create a test that fails before the change and passes after? I'd like to make sure an issue like this doesn't sneak in again.

[notmandatory]

Would it also make sense to fix TxGraph::insert_txout() to not insert duplicates? Since it's a public function I'm worried end users could make the same mistake and insert duplicate txouts and get the same miscalculation. The real problem may be deeper down in the TxGraph::apply_update() call which I believe should prevent creating duplicates in the TxGraph.txs map.

[ValuedMammal]

As I understand the behavior of TxGraph, only the first txout inserted will be kept and attempting to insert another one at the same outpoint just returns an empty changeset. In the integration test for electrum the outpoint we're looking for will always have a vout of 0 (they are coinbase txs), so calculating the fee of the received tx happens to work by sheer luck. A more robust test would involve controlling the utxo spent by Core and in which order it appears in the tx which is posing more of a challenge.

[evanlinjin]

Damn this is a huge oversight by @LagginTimes and myself. Thanks for taking this on.

[LagginTimes]

ACK af15ebb

[evanlinjin]

We definitely need more tests for this. We can add these tests in subsequent PRs.

I propose a separate test specific to testing fee calculation that takes into account what @ValuedMammal mentioned in #1443 (comment):

In the integration test for electrum the outpoint we're looking for will always have a vout of 0 (they are coinbase txs), so calculating the fee of the received tx happens to work by sheer luck. A more robust test would involve controlling the utxo spent by Core and in which order it appears in the tx which is posing more of a challenge.

[evanlinjin]

ACK af15ebb