Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: Wrong Fee Warning #162

Open
andreasgriffin opened this issue Sep 13, 2024 · 12 comments
Open

Bug: Wrong Fee Warning #162

andreasgriffin opened this issue Sep 13, 2024 · 12 comments

Comments

@andreasgriffin
Copy link

andreasgriffin commented Sep 13, 2024

The 0 fee testnet PSBT cHNidP8BAF4BAAAAAReS33YxPXaFQsnndIkSW6rXL5qjximUqUlRo5yq67YjAQAAAAD9////ASQTAAAAAAAAIgAge7kf9Bn5AjXxXIaUPgkHt81IDcInF3nGuawbAulxGDwXUywAAAEA/QkBAgAAAAABARbYhfU+6jNGNk9CSQd2ktWkXldHCatbTNoQQOyRhE1dAAAAAAD9////AyQTAAAAAAAAFgAUdROCdhbgqv+zJ9skc4y0oNWW+oAkEwAAAAAAACIAIDF0NDYGhla3sq1SnTHsqqPPqo3/0ntmq7KZSwSfo6XPbv+uAwAAAAAWABS2GpcEAY9JtMlJ3R7n168rMNjbMAJHMEQCIGgxCUnE3l3AKO4ow5IcYaAgMBkSKm3G1uPdsSUfe4NqAiBL4VdAVj3biWNdOmBsDJImWUCLUdQzn2c9JLGvTcTWIwEhAmWUwOILiCvT0aa8yCYbiM3bv6AqS9JNNpPyS9f0x7OwdlEsAAEBKyQTAAAAAAAAIgAgMXQ0NgaGVreyrVKdMeyqo8+qjf/Se2arsplLBJ+jpc8BBWlSIQLO4pyoXCFhzco+eHYJ2t5PeC6FQEFLW5vvpabuAH2ULyEC4KJjDZmFl1dWLGTp5NwL8h1QEfOCuWxCqw1BRsRAnIYhAwcehaea8oJ14IP1qj77K1bQjIwSGixfYUCYnqmlhOCQU64iBgLO4pyoXCFhzco+eHYJ2t5PeC6FQEFLW5vvpabuAH2ULxzVtDVAMAAAgAEAAIAAAACAAgAAgAAAAAAAAAAAIgYC4KJjDZmFl1dWLGTp5NwL8h1QEfOCuWxCqw1BRsRAnIYcFMlJtDAAAIABAACAAAAAgAIAAIAAAAAAAAAAACIGAwcehaea8oJ14IP1qj77K1bQjIwSGixfYUCYnqmlhOCQHNjPdHUwAACAAQAAgAAAAIACAACAAAAAAAAAAAAAAQFpUiECCPVuZMLXHQr6+8FmS+uulT6Mt9AlDSg9X2fAC1T8HlshAiqNC+U+vJncOwrmJWrzksn6yTT0EQPW48PTwkaghPlFIQMgxxF62RTvu7DKVnak5kI+UkyFFFNrSvZLbWiywaeoUlOuIgICCPVuZMLXHQr6+8FmS+uulT6Mt9AlDSg9X2fAC1T8Hlsc2M90dTAAAIABAACAAAAAgAIAAIAAAAAAAgAAACICAiqNC+U+vJncOwrmJWrzksn6yTT0EQPW48PTwkaghPlFHBTJSbQwAACAAQAAgAAAAIACAACAAAAAAAIAAAAiAgMgxxF62RTvu7DKVnak5kI+UkyFFFNrSvZLbWiywaeoUhzVtDVAMAAAgAEAAIAAAACAAgAAgAAAAAACAAAAAA==

results in a false warning message
d

The false warning message also appears for non-zero low fees.

@JamieDriver
Copy link
Collaborator

If I send the above psbt to Jade I don't see the warning (fw 1.0.31, btw).

cHNidP8BAF4BAAAAAReS33YxPXaFQsnndIkSW6rXL5qjximUqUlRo5yq67YjAQAAAAD9////ASQTAAAAAAAAIgAge7kf9Bn5AjXxXIaUPgkHt81IDcInF3nGuawbAulxGDwXUywAAAEAnAIAAAABFtiF9T7qM0Y2T0JJB3aS1aReV0cJq1tM2hBA7JGETV0AAAAAAP3///8DJBMAAAAAAAAWABR1E4J2FuCq/7Mn2yRzjLSg1Zb6gCQTAAAAAAAAIgAgMXQ0NgaGVreyrVKdMeyqo8+qjf/Se2arsplLBJ+jpc9u/64DAAAAABYAFLYalwQBj0m0yUndHufXrysw2NswdlEsAAEBKyQTAAAAAAAAIgAgMXQ0NgaGVreyrVKdMeyqo8+qjf/Se2arsplLBJ+jpc8BBWlSIQLO4pyoXCFhzco+eHYJ2t5PeC6FQEFLW5vvpabuAH2ULyEC4KJjDZmFl1dWLGTp5NwL8h1QEfOCuWxCqw1BRsRAnIYhAwcehaea8oJ14IP1qj77K1bQjIwSGixfYUCYnqmlhOCQU64iBgLO4pyoXCFhzco+eHYJ2t5PeC6FQEFLW5vvpabuAH2ULxzVtDVAMAAAgAEAAIAAAACAAgAAgAAAAAAAAAAAIgYC4KJjDZmFl1dWLGTp5NwL8h1QEfOCuWxCqw1BRsRAnIYcFMlJtDAAAIABAACAAAAAgAIAAIAAAAAAAAAAACIGAwcehaea8oJ14IP1qj77K1bQjIwSGixfYUCYnqmlhOCQHNjPdHUwAACAAQAAgAAAAIACAACAAAAAAAAAAAAAAQFpUiECCPVuZMLXHQr6+8FmS+uulT6Mt9AlDSg9X2fAC1T8HlshAiqNC+U+vJncOwrmJWrzksn6yTT0EQPW48PTwkaghPlFIQMgxxF62RTvu7DKVnak5kI+UkyFFFNrSvZLbWiywaeoUlOuIgICCPVuZMLXHQr6+8FmS+uulT6Mt9AlDSg9X2fAC1T8Hlsc2M90dTAAAIABAACAAAAAgAIAAIAAAAAAAgAAACICAiqNC+U+vJncOwrmJWrzksn6yTT0EQPW48PTwkaghPlFHBTJSbQwAACAAQAAgAAAAIACAACAAAAAAAIAAAAiAgMgxxF62RTvu7DKVnak5kI+UkyFFFNrSvZLbWiywaeoUhzVtDVAMAAAgAEAAIAAAACAAgAAgAAAAAACAAAAAA==

ofc I don't have the same wallet/seed as you, so I do get a warning about there not being any inputs for me to sign, but that is as expected.

If you can reproduce on testnet, it'd be ideal if you could do so in a temporary wallet where you can share the seed phrase.

(Note the definition of 'send amount' is 'outputs that are not change' - but from looking at the psbt the outputs are not to change bip32 paths, so that shouldn't be relevant [more fyi]).

@andreasgriffin
Copy link
Author

ok. on testnet, funded with a few sats

The following descriptor:

wsh(sortedmulti(2,[45a9be94/48'/1'/0'/2']tpubDEpkV8PcqfxmyRTZHGxWgUJBfSuQvi47otVB7rjvFUJap6DF8QEy7q6uo5XDtL7rUemJvDeB1shGcAVmmZ9iMeNrziqx8nTyKjokPcoj272/<0;1>/*,[30b8300e/48'/1'/0'/2']tpubDEqwba9CCfjDF8iEtx6xV1FzgGW6qUWQx1PdXswoNrY4AeCbmkWfdxZYsDqNP6VU4LSBD6wxfYXDmJ7UK98W5UreEXNz1H7DzktPWVYmQww/<0;1>/*,[394c395b/48'/1'/0'/2']tpubDF9eJKXpDXKCSEg9hDgiQ4HhiBXbpc2k5VVeLpKR2UP61L4Km5xtsPRZZ46JRegGkxpmvDmPTMVSjRhxpxHiJFj2mRcBsLpHqXuVqoSzdpL/<0;1>/*))#pcjqddc7

the following jade seed:

garment donkey split oppose rent pet office april grow again hawk evidence

the psbt:

cHNidP8BAF4BAAAAAaDVmaz5RvhybHusbcwtj7BUMfIvOhuvAdBs0z/YaLj5AAAAAAD9////AWoPAAAAAAAAIgAgSrr1L48byGG8+dtj7aU5S+7XjiSbKs6SM4yTqFYlL3KSpSwATwEENYfPBHzUlz6AAAACQWUorVFrxMIsO+IwGReke63wK1oOQUalJ2RQ5xBxgaoCKTc2l6cfghjEFjl0Sv/MAWoOvQ1EGzmRK9zi9/uoaUIURam+lDAAAIABAACAAAAAgAIAAIBPAQQ1h88Ef6Aha4AAAAI5C4dia+KR+JVM0lNxz8OYkoUiPo+x4gUH6WIN6P42dQNp0fljymO5TRZdbwS/O1cc0q4FEnkFwdXwvg+RFJBDQRQwuDAOMAAAgAEAAIAAAACAAgAAgE8BBDWHzwShCT+FgAAAAqrgps8heyaTCzT7P/2sdOxa6e9zz+cx7kEahtFDvsrHAmVfDy3tpHqeaFib3eAnOgC5pA8qsoQYeG7da7NTlA0CFLc9iccwAACAAQAAgAAAAIACAACATwEENYfPBKkmEJqAAAAC+tZjgJ95Agd8s3rnc5wTdKCbbemRo2MGk2cLr+d2QqQDPd7eC9yuk+6+sFpYrWx6l1QM5xt5MXj0AgZf2rXhFOUUOUw5WzAAAIABAACAAAAAgAIAAIBPAQQ1h88E8fAhO4AAAALWK+1nSLbMBn+ty4ArGBmqXkgGAQ7XgWcRmGw+Cao4DQL3WvfeQ5an/If7nSgXQWA8Yq1maXVRZFThyTpn7ujhThT1WR7sMAAAgAEAAIAAAACAAgAAgE8BBDWHzwT+5qJ5gAAAAuGbAETMrOE7JvEF41iLtSZeuAkn3uDMJw0iopoXUMDDAggEdt17QSy8Rwc24IgY2nCBqxMo7KDpSFd84jLphAd9FNW0NUAwAACAAQAAgAAAAIACAACAAAEA/VwBAQAAAAABATNFpkr5LG00rn3EpngL6z7TadKcfjdRKpTkPXMKggA9AAAAAAD9////AWoPAAAAAAAAIgAgOyvNy6yTqgNDws4XHhM2Wf0TAjWjWq3uG1qjsuBLJusEAEcwRAIgBVFo3mYZsuAvs7xyNquyZHRC4kGWKbQpFtcLhoZoRx8CIAfwE8E7FGcz+DxU1dGGqzM23y3kZnEtEuzAWvwj3gcMAUcwRAIgLk44GNFvEuFQ56lQOksrViPknAgHDE6srLrlNqGYKxICIAa97qYClAw/G73QiyGg1myGBw0P4QqTyjAcbl/njYwyAWlSIQMNqe9Xx5q/BmV1osH8rsFbnpB+7JiIpDpUK1TrkVdw7iEDf1iIv+q18JyQfWSF18ruSwA6VmSyyCyZFCgwS7E5p8YhA/4hqlStD4GVlP/vJ3YvcKMyQtby/dXDa8UjI+GdVSXwU66SpSwAAQErag8AAAAAAAAiACA7K83LrJOqA0PCzhceEzZZ/RMCNaNare4bWqOy4Esm6wEFaVIhAy+J3qzzX4GV+TA1ELko/uQM1u0fpTIbd1Yumq97RQiwIQOKsDN5KAJew8IZMQdO0NBScG5CFlpfR7WQPjl93iME5CEDwh0sFDFqw1KGG8jHbslgykTtVrhvnxX2z2C4+BNq34JTriIGAy+J3qzzX4GV+TA1ELko/uQM1u0fpTIbd1Yumq97RQiwHEWpvpQwAACAAQAAgAAAAIACAACAAAAAAAAAAAAiBgOKsDN5KAJew8IZMQdO0NBScG5CFlpfR7WQPjl93iME5Bw5TDlbMAAAgAEAAIAAAACAAgAAgAAAAAAAAAAAIgYDwh0sFDFqw1KGG8jHbslgykTtVrhvnxX2z2C4+BNq34IcMLgwDjAAAIABAACAAAAAgAIAAIAAAAAAAAAAAAABAWlSIQJwWM5A5pRJhBeOVvrf4mGHUTdNEsFWuIYNcDXSABFMASEDluQKIse7rn5bNe2OXl48EWwHQRkgGugHl/zckrWLXw8hA8JztYLtsL1OpGPln0mf3U+Esq1MBhIS+YE0zrCed6iJU64iAgJwWM5A5pRJhBeOVvrf4mGHUTdNEsFWuIYNcDXSABFMARxFqb6UMAAAgAEAAIAAAACAAgAAgAAAAAABAAAAIgIDluQKIse7rn5bNe2OXl48EWwHQRkgGugHl/zckrWLXw8cOUw5WzAAAIABAACAAAAAgAIAAIAAAAAAAQAAACICA8JztYLtsL1OpGPln0mf3U+Esq1MBhIS+YE0zrCed6iJHDC4MA4wAACAAQAAgAAAAIACAACAAAAAAAEAAAAA

gives me the described warning

@JamieDriver
Copy link
Collaborator

Did you register that descriptor on Jade (if so, how ... using some companion app?)

@JamieDriver
Copy link
Collaborator

JamieDriver commented Sep 17, 2024

ok, I used the attached script:

psbt_fee_test.py.txt

When signing I see:

  • "Output 1/1" of "0.00003946 BTC" with a message beneath telling me it is a "Verified wallet output" - ie. that Jade can confirm the output is back to the same wallet as the input being signed (eg. is a redeposit/consolidation).
  • The "Send Transaction" screen with the fee of "0.00000000 BTC" - with no 'high fees' warning.

I think this is what I'd expect to see.

What fw version are you using ?

@JamieDriver
Copy link
Collaborator

JamieDriver commented Sep 17, 2024

NOTE: if the descriptor wallet is not registered on Jade, ie. 'Step 3.' is:

    # 3. Register descriptor wallet
    #rslt = jade.register_descriptor(NETWORK, DESC_NAME, DESC_SCRIPT, DESC_KEYS)
    #assert rslt

    # Check loaded descriptor
    registered_descriptors = jade.get_registered_descriptors()
    desc = registered_descriptors.get(DESC_NAME)
    assert not desc
    #assert desc
    #assert desc['descriptor_len'] == len(DESC_SCRIPT)
    #assert desc['num_datavalues'] == len(DESC_KEYS)

    #desc = jade.get_registered_descriptor(DESC_NAME)
    #assert desc
    #assert desc['descriptor'] == DESC_SCRIPT
    #assert desc['datavalues'] == DESC_KEYS

(and I use the 'Options->Wallet' menu to ensure that registration is deleted...)

Then I no longer see the "Verified wallet output" message (as Jade does not have sufficient information about the cosigners to be able to assert this) but I still do not see a 'high fees' warning.

@andreasgriffin
Copy link
Author

andreasgriffin commented Sep 29, 2024

What fw version are you using ?

1.0.29

I will update to 1.0.31 and see if the bug is there still.

@andreasgriffin
Copy link
Author

In 1.0.31 the error message does not appear

@andreasgriffin
Copy link
Author

andreasgriffin commented Sep 29, 2024

However for another PSBT the warning is still there (also with 0 fee):

PSBT: cHNidP8BAFIBAAAAAZsCKtC6UTSN/+4e3jnWiYyv0C7WYNGsOSdnNBWMU7y4AAAAAAD9////AdAHAAAAAAAAFgAU43t9/p+a2e2F8hB9+o5qYf5E6B255S0ATwEENYfPA2VmPE2AAAAAVtGT3N/zqb7CxcYIvWym7G7fFKsHqxld11mIdb31+KsD9OsPX3Due4Kn7npJ9Mk00OLW6t+2IiBvGL9jQls65iUQMLgwDlQAAIABAACAAAAAgAABAP1yAQEAAAAAAQJjlQsOh7WHbaNwmiWWwiPdE2E6jZ5qTpUcxhI5H0sjCQAAAAAA/f///zfYJ3KeEmhHnlWjLMQuKF2Jp5MZ3x8xNTx9mj2xWdQwAQAAAAD9////AtAHAAAAAAAAFgAU+7C6uZ/CKy5T0B8t4uj4P7Hxv0qgDwAAAAAAABYAFDTCvNGvZ2dAKDg6OGiQnxg5nsCsAkcwRAIgDYy81/lDTm5jI/1hazC9BOuI9mdTiwOK3EO1YmszbToCIB3xSW4H9JaOz2xNFPid+mtTFKpHzy7TeAJO/HAF1ziWASECYcxwU8waQTtyRR+TuQ5NJ+EaYJ30iAoff+ZqwjMO27oCRzBEAiB4/sBbRNnqGn9r21kTKSRjCDyIpxZd+8e0rotOf3G41wIgL5PMb5LIW7YGXaE36Y0EimnfqH2WFpASvdTGXBC1zEsBIQL+q48kJi5W+bQNMIUrJbrXpnh847CjGJ5jVyuaINUTybflLQABAR/QBwAAAAAAABYAFPuwurmfwisuU9AfLeLo+D+x8b9KIgYDFr55KGENkOM4dmVR+33t/oVX+lT9zMxbnyJUy6a2bScYMLgwDlQAAIABAACAAAAAgAAAAAAAAAAAACICAn0T361AbL8Cckns3wSUkg8GN7Yd0/wZWYyZ4R35E5rUGDC4MA5UAACAAQAAgAAAAIAAAAAAFAAAAAA=

using the same seed as above.

image

@JamieDriver
Copy link
Collaborator

JamieDriver commented Sep 30, 2024

If I substitute the above psbt into the script given above, I still do not see a warning, so I don't know how you are triggering it. Somehow it must think you're not spending anything...

The comparison that triggers that warning is essentially: fees >= spend_amount.

I could change to ( for example ): fees > 0 && fees > spend_amount, which should make it slightly less sensitive, and mean it never pops up when fees are zero (which seems sensible).

@JamieDriver
Copy link
Collaborator

My suspicion is that jade is not being passed this psbt, but rather it is being passed to something like HWI/hwilib (or similar), which is translating it into Jade's legacy format (there is an open PR on HWI to pass psbt through as is).
In any case I think that might be marking all outputs back to the spending wallet as 'change', and therefore Jade would think the 'spend' is zero.
This case would be addressed by the 'fees > 0' check above.

@andreasgriffin
Copy link
Author

andreasgriffin commented Oct 1, 2024

Yes, I use HWI (see https://github.com/andreasgriffin/bitcoin-usb/ ) . I was unaware there is a Jade legacy format.
fees > 0 will not be sufficient, because the warning also happens for low fee transactions.

@JamieDriver
Copy link
Collaborator

JamieDriver commented Oct 1, 2024

There's nothing I can do about that until HWI is fixed.
When you say 'transactions', I assume you mean 'transactions where I send coins only to myself' ?

Where you send funds back to your own wallet they can be categorised as 'spend' or 'change' (coins to external addresses are only ever 'spend'). HWI is telling Jade that all the funds to your own wallet are "change" and therefore there is no "spend" amount. In this case any fee will be > 'spend' (since 'spend' is 0), so the warning will be shown.
Either a) HWI should have a better idea of when coins to self are 'spend' and when they are 'change', or b) HWI can just use the PSBT format - in which case it doesn't have to tell Jade which outputs are spend and which are change, as Jade deduces that for itself based on the bip32 path.
(The fix for 'a' would be easy enough, but the fix for 'b' is already open and is a 'better' approach anyway ...)

fees > 0 is still a reasonable fix to make - there's no point saying 'the fees look a bit high!' when in fact it's a zero fee! ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants