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

Add liquid and liquid_regtest #285

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

miketlk
Copy link

@miketlk miketlk commented Nov 7, 2024

This PR is about adding support for the new Liquid Network application in two variants: "liquid" and "liquid_regtest".

Please pay attention to the "path_slip21" field that I have defined as:

"path_slip21": ["LEDGER-Wallet policy", "SLIP-0077"]

As I understand, using an array here would be non-standard, since all other apps define at most a single path. But I don't have a better idea, as the application really needs two SLIP-0021 paths for normal operation. The first one, "LEDGER-Wallet policy" is used to register non-standard wallets in the exactly same way as Bitcoin app does. The Liquid app is based on it and inherits this mechanism as is. Another path, "SLIP-0077" is used to derive the master blinding key from the seed.

Another potential issue is with the "path" field. Currently, it is defined in the same way as for the Bitcoin app:

"path": [null]

This is definitely not the best way from the side of potential security risks, as it grants the app the right to derive any possible key from the seed. I'd prefer to specify a more restricted list of paths, like:

"path": ["44'/1'", "48'/1'", "49'/1'", "84'/1'", "86'/1'"]

However, with the current SDK, AFAIK, there is no dedicated function returning master key fingerprint. So, like in Bitcoin app, it is computed on the app side, requiring to derive the root pubkey.

@miketlk
Copy link
Author

miketlk commented Nov 13, 2024

I removed "LEDGER-Wallet policy" from "path_slip21" to make this PR mergeable. However, it would be great if "path_slip21" field could support more than one path. As a temporary solution, I'm considering using a modified SLIP-0021 scheme to derive the wallet HMAC key, in which the seed is substituted with a child private key derived at some dedicated path like m/12345678'/87654321'.

@miketlk
Copy link
Author

miketlk commented Nov 26, 2024

It looks like that the check "Check if PR branch contains merge commits" in the force-rebase workflow has a false positive error termination. As far as I understand, this step should check that there is at least one merge commit in the branch, but this condition should be satisfied with 84bf78d.

Should I create an issue?

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

Successfully merging this pull request may close these issues.

1 participant