The testing client imports unspendable inputs

[manor]

Affected Branch

trunk

Basic Diagnostics

• [X] I've pulled the latest changes on the affected branch and the issue is still present.

• [X] The issue is reproducible in docker

Description

root@2af70284b1b5:/opt/tx-processor# ./build/src/uhs/client/client-cli 2pc-compose.cfg mempool0.dat wallet0.dat mint 1 1 [2022-02-13 03:59:18.147] [WARN ] Existing wallet file not found [2022-02-13 03:59:18.147] [WARN ] Existing client file not found fdcc8e5559af55040a4f0f12db2b4de5f614ffaf08d359d727b3b07c12a6c64f root@2af70284b1b5:/opt/tx-processor# ./build/src/uhs/client/client-cli 2pc-compose.cfg mempool1.dat wallet1.dat newaddress [2022-02-13 03:59:54.151] [WARN ] Existing wallet file not found [2022-02-13 03:59:54.152] [WARN ] Existing client file not found usd1qpsctc3vzrwa2e2ru0lehgjh8p5l8y2cklsuh6sd6grr3n6eqayn7r9zgq8 root@2af70284b1b5:/opt/tx-processor# ./build/src/uhs/client/client-cli 2pc-compose.cfg mempool0.dat wallet0.dat send 1 usd1qpsctc3vzrwa2e2ru0lehgjh8p5l8y2cklsuh6sd6grr3n6eqayn7r9zgq8 tx_id: f75737d445cdf02ba8ca7cf6372930cc974c7e9cbc2ef3fc814b379772cdecf5 Data for recipient importinput: f75737d445cdf02ba8ca7cf6372930cc974c7e9cbc2ef3fc814b379772cdecf50000000000000000e5ad0c7243ec63ff655e959f7ed68be9eebec3c1b38e1c11956a37207956f5f00100000000000000 Sentinel responded: Confirmed root@2af70284b1b5:/opt/tx-processor# ./build/src/uhs/client/client-cli 2pc-compose.cfg mempool1.dat wallet1.dat importinput f75737d445cdf02ba8ca7cf6372930cc974c7e9cbc2ef3fc814b379772cdecf50000000000000000e5ad0c7243ec63ff655e959f7ed68be9eebec3c1b38e1c11956a37207956f5f00100000000000000 root@2af70284b1b5:/opt/tx-processor# ./build/src/uhs/client/client-cli 2pc-compose.cfg mempool0.dat wallet0.dat importinput f75737d445cdf02ba8ca7cf6372930cc974c7e9cbc2ef3fc814b379772cdecf50000000000000000e5ad0c7243ec63ff655e959f7ed68be9eebec3c1b38e1c11956a37207956f5f00100000000000000 root@2af70284b1b5:/opt/tx-processor# ./build/src/uhs/client/client-cli 2pc-compose.cfg mempool1.dat wallet1.dat sync root@2af70284b1b5:/opt/tx-processor# ./build/src/uhs/client/client-cli 2pc-compose.cfg mempool0.dat wallet0.dat sync root@2af70284b1b5:/opt/tx-processor# ./build/src/uhs/client/client-cli 2pc-compose.cfg mempool1.dat wallet1.dat info Balance: $0.01, UTXOs: 1, pending TXs: 0 root@2af70284b1b5:/opt/tx-processor# ./build/src/uhs/client/client-cli 2pc-compose.cfg mempool0.dat wallet0.dat info Balance: $0.01, UTXOs: 1, pending TXs: 0

Code of Conduct

• [X] I agree to follow this project's Code of Conduct

[HalosGhost]

I am able to reproduce this locally. I have a hunch on what's happening here. This looks like a bug in our testing client that results in importing an input regardless of whether or not that input is actually spendable.

For example, after performing the above operation, the recipient of this first transaction is able to spend the new single-cent token freely, but if the initial sender tries to send the cent it thinks it has, it will receive this error from the sentinel (because the client cannot produce a valid witness for the input):

root@96e30c314841:/opt/tx-processor# ./build/src/uhs/client/client-cli 2pc-compose.cfg mempool0.dat wallet0.dat send 1 usd1qzmc97ugt365us6sj58tgn9rav4jcjrndlzw28q2n62s57dewcfhqw2hpuc
tx_id:
beb23d934d6a13bec6078cb764168ece5c3b4068a3efc5fe2e6a5e514eaafdb0
Data for recipient importinput:
beb23d934d6a13bec6078cb764168ece5c3b4068a3efc5fe2e6a5e514eaafdb00000000000000000296c16663ef530eccf8b244534b29d6c37a512f2ca3d4eef305f17bc11722abe0100000000000000
Sentinel responded: Statically invalid
Validation error: Witness error (idx: 0): Incorrect witness data length

In short, this does not appear to actually allow the creation of funds out of thin air; but does mean that our testing client is capable of being fooled into thinking it has funds which it doesn't actually control.

[HalosGhost]

In looking a little deeper into this, my hunch was indeed correct.

In particular, you can see that client::import_send_input immediately imports an input without checking to see if it's spendable. This isn't particularly bad-behavior for a testing client, but it's easy-to-fix.

A check should probably be added transaction::wallet::update_balance ensuring that the inputs could actually be spent before being inserted.

Similarly, it probably makes sense to add a new method (e.g., wallet::is_spendable(transaction::input&) -> bool) so that client::import_send_input() can check if the input is spendable before even trying to confirm it.

I've updated the issue's title, and will be relabeling it in case anyone would like to dive in and fix this!