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

Support mainnet or testnet addresses on any derivation path #52

Closed
kyranjamie opened this issue Jan 22, 2021 · 9 comments · Fixed by #55
Closed

Support mainnet or testnet addresses on any derivation path #52

kyranjamie opened this issue Jan 22, 2021 · 9 comments · Fixed by #55
Assignees
Labels

Comments

@kyranjamie
Copy link
Contributor

I don't believe there's an immediate rush for this feature, however inline with the expected behaviour of our addressing conventions, we do need to be able to specify network in addition to derivation path.

This could be in the form of an additional flag passed to the device.

@kyranjamie
Copy link
Contributor Author

To add to this, I don't believe I'm able to generate testnet address even with the testnet path. Using m/44'/1'/0/0/0 still returns mainnet addresses.

@neithanmo
Copy link
Collaborator

Hi @kyranjamie. so if I understood well, we want to generate a testnet address for any derivation path. Your idea is passing a flag for such a case, right? can you elaborate a bit more?

@kyranjamie
Copy link
Contributor Author

Hi @neithanmo, yeah that's pretty much it. From a consumer point of view:

Here's what we have now:

sign(path: string, message: Buffer): ResponseSign

here's what we're after

sign(path: string, network: 'testnet' | 'mainnet', message: Buffer): ResponseSign

Stacks doesn't follow BIP-44 exactly, which is why we need this behaviour.

@neithanmo
Copy link
Collaborator

neithanmo commented Feb 19, 2021

This flag or network information is already part of the transaction structure,, isn't it?
so with this new flag, should we ignore the one in the transaction?

@kyranjamie
Copy link
Contributor Author

Good point. If we can infer chain from the tx, and keep the API as it is now, that would be great.

@neithanmo
Copy link
Collaborator

I think, this line sets the transaction version number. I did the next two experiments:

  • const network = StacksTestnet, and same derivation path
    I got:
    image

then:

  • const network = StacksMainnet(), keep using the same path at it is in the example, and got:
    image

Not sure If this is the expected behavior. I guess ST stands for testnet and SP for mainnet ??

@kyranjamie
Copy link
Contributor Author

Sorry @neithanmo sign wasn't particularly demonstrative of this issue.

Consider the address generation methods, such as showAddressAndPubKey. Currently it only accepts a derivation path. Ideally we need a signature like showAddressAndPubKey(path: string, network: 'testnet' | 'mainnet').

@neithanmo
Copy link
Collaborator

Ok, I see that function. Yes, It seems that we can not generate a testnet address there. I am not well versed in JS, but maybe we can define an overloaded function, and have another backend function on the ledger internal side, that allows us to pass in this flag. let me dig a bit more at what we have internally, and how we can overcome this issue.

@neithanmo
Copy link
Collaborator

as an update. #55 is a wip that addresses this.

@jleni jleni linked a pull request Mar 5, 2021 that will close this issue
@jleni jleni closed this as completed Mar 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants