-
-
Notifications
You must be signed in to change notification settings - Fork 5
feat: bitcoin account types #284
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
base: main
Are you sure you want to change the base?
Conversation
...KeyringAccountStruct.schema, | ||
|
||
/** | ||
* Account address. | ||
*/ | ||
address: BtcP2wpkhAddressStruct, | ||
address: string(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should still validate the address formats here. We could use bitcoin-address-validation
package for this, however, given that we are using 1 single struct, it will be hard to make sure the address
is valid for the given type
.
So maybe we should have 4 different structs instead (similarly to BtcP2wpkhAccountStruct
, we would have BtcP2shAccountStruct
, etc...), allowing us to strictly check the address
is valid for the account's type
?
WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The address should not be validated because for the extension, the address is a string
. For example if the Snap tells you that your address is fooBar
then that's your address. you can't decide that you don't agree with the Snap. You can very well imagine that we are using petnames
instead of bech32
addresses in the Snap and it would be acceptable.
Knowing the address encoding is not needed in the business logic. We can validate length though.
Co-authored-by: Charly Chevalier <[email protected]>
Add support for remaining Bitcoin account types.
This PR removes validation checks for the decoding of the address as the address is never decoded and simply displayed as returned from the Snap. Removes
bech32
dependency.p2sh
is a shortcut forp2wpkh-p2sh
(nested segwit).