elements icon indicating copy to clipboard operation
elements copied to clipboard

Getwalletinfo does not return unconfirmed_balance for assets

Open FrancisPouliot opened this issue 5 years ago • 7 comments

The getwalletinfo RPC call returns

{ "walletname": "", "walletversion": 169900, "balance": { "L-CAD": 0.xxxxxxxx, "bitcoin": 0.xxxxxxxx }, "unconfirmed_balance": { "bitcoin": 0.00000000 }, "immature_balance": { "bitcoin": 0.00000000 }, "txcount": 14, "keypoololdest": 1580930368, "keypoolsize": 1000, "keypoolsize_hd_internal": 999, "paytxfee": 0.00000000, "hdseedid": "XXXXXXXXXXXXXX", "private_keys_enabled": true }

It would really be helpful to have the unconfirmed_balance for per assetID as well.

FrancisPouliot avatar Feb 24 '20 16:02 FrancisPouliot

First of all, those values have been deprecated upstream in commit facfb4111d14a3b06c46690a2cca7ca91cea8a96 (probably version 0.19), so we might soon deprecate them as well.

"  \"balance\": xxxxxxx,                (numeric) DEPRECATED. Identical to getbalances().mine.trusted\n"
"  \"unconfirmed_balance\": xxx,        (numeric) DEPRECATED. Identical to getbalances().mine.untrusted_pending\n"
"  \"immature_balance\": xxxxxx,        (numeric) DEPRECATED. Identical to getbalances().mine.immature\n"

That being said, I'm a bit confused by your example. Are you saying that the unconfirmed_balance of a certain asset is non-zero and missing? Or that it's zero and you would like us to explicitly give a value of zero?

It seems to me that it's the latter. We can't explicitly report zero balances of all existing assets, simply because the daemon doesn't know about all existing assets. We know about the Bitcoin asset because it's a special-case asset in which fees are paid. There might be an argument to be made for also listing zero entries for the assets that have a non-zero entry in the balance map, though. But personally I don't really see how any 0 value is useful. You can simply check if your asset id is in the map and if not, it's 0...

stevenroose avatar Feb 25 '20 12:02 stevenroose

For this example, I specifically tested for a balance was non-zero, and it was missing.

FrancisPouliot avatar Feb 25 '20 13:02 FrancisPouliot

Ok so I write a test to check some scenarios: https://github.com/ElementsProject/elements/pull/828

It seems that the only problem there is is when you're just issueing an asset, the balance goes into confirmed state directly even though the issuance tx is still unconfirmed. While that's wrong, it's harmless because at that point the asset really only exists in that unconfirmed transaction.

This means that your example must come from right after calling issueasset, is that right?

In the test I wrote, just sending assets work correctly when it comes to showing unconfirmed balances.

stevenroose avatar Feb 26 '20 11:02 stevenroose

https://github.com/ElementsProject/elements/blob/master/src/wallet/wallet.cpp#L2280

Self-sends are trusted, and "unconfirmed" balance is actually trusted.

In 0.19 and beyond the non-deprecated balance check is getbalances which has much better naming.

instagibbs avatar Feb 26 '20 14:02 instagibbs

@FrancisPouliot It looks like what's happening is intended behavior. The wallet considers transactions that are sent by the user as "trusted" automatically. And "unconfirmed" actually should be "untrusted".

In Core 0.19, this is all fixed by deprecating confirmed<>unconfirmed balances and calling them trusted<>untrusted.

So the issueasset shows up in confirmed and not in unconfirmed because it is sent by the user itself and it thus won't be double spent.

stevenroose avatar Feb 26 '20 14:02 stevenroose

Oh yes, I actually tested from my own wallet to my own wallet. Good catch

So Liquid is going to update i'm assuming is the point?

FrancisPouliot avatar Feb 26 '20 22:02 FrancisPouliot

Yeah we're working on catching up with upstream v0.19 so that we can get Elements also to that line. Which will inherit the same breaking changes and deprecations that Core introduced.

On Wed, Feb 26, 2020 at 10:49 PM Francis Pouliot [email protected] wrote:

Oh yes, I actually tested from my own wallet to my own wallet. Good catch

So Liquid is going to update i'm assuming is the point?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ElementsProject/elements/issues/827?email_source=notifications&email_token=AAGQLXCFHBAC2ZRDTRHJJ3DRE3WW5A5CNFSM4K2LY7P2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOENCGIIY#issuecomment-591684643, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGQLXBJK5AMYI67GSXS7WDRE3WW5ANCNFSM4K2LY7PQ .

stevenroose avatar Feb 27 '20 11:02 stevenroose