rpc/wallet: add optional transaction(s) to getbalances #22776

Optional transactions provided to getbalances iterates over the inputs and outputs, and tallies up the balance change for the given wallet. This can be useful e.g. when verifying that a coin join like transaction doesn't contain unexpected inputs that the wallet will then sign for unintentionally.

This is an alternative to #22751.

Concept ACK. Will try today.

Approach ACK, definitely prefer this over #22751 👍

How is the user going to use this? Call the RPC thrice and compare the results?

• Fist time to get the balances before
• Second time to get the balances with the tx included
• Third time to verify no other thread added a wallet tx in-between, which would invalidate the result

@MarcoFalke The transaction is supposedly not yet broadcasted. They'd only need to call getbalances once. The existing output would give the wallet's balance as it is (excluding the transaction), and the added "changes" would show how the balance would change, if the transaction was broadcasted.

I.e. I get a coin-join with 100 other inputs and a bunch of outputs. I call getbalances [hex], and the "changes" column tells me how much I gain/lose from broadcasting the transaction.

Oh I didn't see the changed result, because the documentation wasn't updated.

Good point, documentation updated.

<!--e57a25ab6845829454e8d69fc972939a-->

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #23813 (Add test and docs for getblockfrompeer by fjahr)
• #23706 (rpc: getblockfrompeer followups by Sjors)
• #23532 (test: add functional test for -startupnotify by brunoerg)
• #22751 (rpc/wallet: add simulaterawtransaction RPC by kallewoof)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

tACK https://github.com/bitcoin/bitcoin/pull/22776/commits/67ff5ae4b092c3284da7481e13518dbbd3d945f5

Tested with different transactions (input from wallet, input not belonging to wallet, outputs from wallet, outputs outside wallet)

Example:

Request:

POST /wallet/W2 HTTP/1.1
Host: 127.0.0.1:18333
Authorization: Basic dXNlcjMzOnBhc3N3b3JkMzM=
Content-Type: text/plain
Content-Length: 521

{"jsonrpc": "1.0", "id": "curltest", "method": "getbalances", "params": ["02000000000101fff8d9d39105eeec162fb45bd2a3e796b255866469a2cc7de93e438f3722c1490000000000fdffffff02801a0600000000001600145b497cfd64324fb0c4f7317472ca591cd7eb3bbfe093040000000000160014b1348ec3acd422b7c84f97f8757a0a5c3db9e32d02473044022020c3a9d7d06429008de82f13de672aac7886c5d6557a7c330bcb8cbc20b8cf4f02203890042ed8bf331e92f07df5d2a7bf5d8ade12cbb216f14493a98d0a188c4918012103c65a6e4a66a4a6b4af981c5e81632bb2d7336bc75d7ccf83e055006a2693050f00000000"]}

Response:

{
    "result": {
        "mine": {
            "trusted": 5051.31588006,
            "untrusted_pending": 0.00000000,
            "immature": 0.00000000
        },
        "tx": {
            "changes": 0.00000000
        }
    },
    "error": null,
    "id": "curltest"
}

Everything looks good to me. Also tested by fuzzing this argument for getbalances with random strings. No issues.

-0 on this.

As I commented in #22751 (comment), I would prefer a broader "see how transactions modify the wallet without actually modifying it" RPC rather than trying to jam that functionality into an existing one.

But if this is the approach that others prefer, then I would also prefer this to accept multiple transactions (as an array) rather than a single transaction. It is conceivable that a user would want to see how multiple transactions would modify their wallet at the same time, e.g. when broadcasting a package.

Switched to array of raw transactions.

I am neutral on whether to add this here or create a new simulaterawtransaction as in #22751.

#22751 seems to make more sense IMO

If users need both pieces of information, they can just batch them.

I think #22751 is emerging as the superior option here. Ping if you disagree and I'll reopen this.

Some prefer this over #22751, so reopening this for now.

I would prefer a broader "see how transactions modify the wallet without actually modifying it" RPC rather than trying to jam that functionality into an existing one.

I tend to agree here. It is useful functionality but I think it's cleaner to have it separated as a seperate RPC.

<!--cf906140f33d8803c4a75a2196329ecb-->

🐙 This pull request conflicts with the target branch and needs rebase.

_{Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft".}

<!--13523179cfe9479db18ec6c5d236f789-->There hasn't been much activity lately and the patch still needs rebase. What is the status here?

• Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
• Is it no longer relevant? ➡️ Please close.
• Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.