bitcoin: display account name when sending to self #1360
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Before, we did this only when sending to the same account. This commit extends this to work when sending to an address of a *different* account of the same keystore. The bitcoin transaction script_configs field is used for inputs and change outputs. We add a similar list, `output_script_configs`, which allows to attach script configs of different accounts to outputs, with `output_script_config_index` to reference them in the output. To ensure backwards compatibility, `output_script_config_index` is added as a new field, which takes precedence over `script_config_index` when set. The output script configs are validated, but since one can send to any address of the same keystore, the output script config validation allows any combination of script configs, unlike the input script configs. To not duplicate lots of code, a few refactors are done: - validate_script_configs is now used for generic validation and is used to validate the output script configs as well as the input script configs - validate_input_script_configs performs additional validation as before (can't mix multisig and single sig inputs, can't mix inputs from different accounts, etc). - the multisig/policy name is stored in ValidatedScriptConfig, so we don't have to fetch it twice The Python lib version was downgraded from 8.0.0 to 7.0.0, it seems it was bumped before in error - the previous release tag and release is 6.3.0.
benma
commented
Jan 30, 2025
| // from BTCSignInitRequest. This enables verification that an output belongs to the same keystore, | ||
| // even if it is from a different account than we spend from, allowing us to display this info to | ||
| // the user. | ||
| optional uint32 output_script_config_index = 9; |
There was a problem hiding this comment.
Wondering it this should just be optional BTCScriptConfigWithKeypath output_script_config instead of a reference into a vector, to simplify the code and API of client libraries.
The script_configs vector for inputs/changes is there so
- we can validate that multisig/singlesig inputs are not mixed and that different accounts are not mixed, and
- that we can confirm multisig details at the beginning of a transaction
- and to have a speedup to not repeat validation for each input
All that is not necessary for the output script infos, and the validation performance is not so important there as the user is anyway manually confirming the output 🤔
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Before, we did this only when sending to the same account. This commit extends this to work when sending to an address of a different account of the same keystore.
The bitcoin transaction script_configs field is used for inputs and change outputs. We add a similar list,
output_script_configs, which allows to attach script configs of different accounts to outputs, withoutput_script_config_indexto reference them in the output.To ensure backwards compatibility,
output_script_config_indexis added as a new field, which takes precedence overscript_config_indexwhen set.The output script configs are validated, but since one can send to any address of the same keystore, the output script config validation allows any combination of script configs, unlike the input script configs. To not duplicate lots of code, a few refactors are done:
The Python lib version was downgraded from 8.0.0 to 7.0.0, it seems it
was bumped before in error - the previous release tag and release is 6.3.0.