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

Wrapping public keys in TS structures before Bech encoding #1261

Closed
joe-u410 opened this issue May 24, 2023 · 10 comments
Closed

Wrapping public keys in TS structures before Bech encoding #1261

joe-u410 opened this issue May 24, 2023 · 10 comments
Assignees
Labels
milestone/genesis Should be done prior to genesis

Comments

@joe-u410
Copy link

const address = Bech32.generateAddress(principal);

Right now, smapp is wrapping public keys in a Typescript template before doing the psuedo-Bech32 encoding used in other parts of the Spacemesh codebase. Because of this, any address generated via the spacemeshos/address or spacemeshos/address-wasm repos directly will not match the address generated by smapp, leading to confusion at best or loss of funds at worst. Happy to discuss further here, but I believe that the correct logic here would just be to feed the pubkey to a bech encoding library directly without wrapping it in this struct.

@lrettig
Copy link
Member

lrettig commented May 24, 2023

Cf. spacemeshos/smcli#38

@brusherru
Copy link
Member

@joe-u410 thank you for being involved in the project and welcome!

I have good news, there is no mistake.
That's how the account abstraction works.

In comparison to EVM-like networks, in Spacemesh VM we're not using the public key as an actual address. Instead of this, you can use your keys for creating and managing different accounts.
In other words, you can use your single keypair (but, of course, it is not recommended) to manage your "simple account" (that can accept and send funds) and also manage your "multisig account" or some kind of vaults. And that's why you have to spawn your account before providing the correct public key.

Also, I've double-checked that Smapp generates key pairs the same as smcli does.
It means that you'll be able to recreate your wallet file from mnemonics and have access to your accounts, using both tools.

So I'm closing the issue.
If you think I miss something — let me know.

@joe-u410
Copy link
Author

Thanks for the response! I'm familiar with BIP 44 style wallet heirarchies. I think we're talking about different issues here. I've also verified that smapp and smcli generate the same public/private keypairs, this issue is referring specifically to how smapp is handling the conversion between the pubkey and the bech-encoded address. Right now, smapp wraps the pubkey in a typescript template before converting to the sm1... style bech address. This means that it will be difficult to maintain similar logic in other repos - how will you guarantee that the pubkey is wrapped with the same padding in Go or Rust address generation?

@lrettig
Copy link
Member

lrettig commented May 31, 2023

Ideally this would live in one place, probably the SDK, and be used everywhere: spacemeshos/spacemesh-sdk#10

@lrettig lrettig reopened this May 31, 2023
@dshulyak
Copy link
Contributor

dshulyak commented Jun 6, 2023

@brusherru could you please summarize what we are doing encoding-wise in smapp

in binary addresses are represented as 24 bytes, 4 first bytes reserved zeroes, next 20 bytes are last 20 bytes from principal address hash

given standard bech32 library the process of encoding is looks like this

dataConverted, err := bech32.ConvertBits(a[:], 8, 5, true)
	if err != nil {
		log.Panic("error converting bech32 bits: ", err.Error())
	}

result, err := bech32.Encode(conf.NetworkHRP, dataConverted)
	if err != nil {
		log.Panic("error encoding to bech32: ", err.Error())
	} 

are we doing the same in smapp or something else?

@brusherru
Copy link
Member

@dshulyak,

  1. First of all, we calculate principal address in Smapp:
    https://github.com/spacemeshos/sm-codec/blob/master/src/transaction.ts#L65-L71

    1. Concatenate TemplateAddress and PublicKey
    2. Do blake3 hashing
    3. Cutting the first 12 bytes (I don't remember why; theoretically, it might be the problem).
    4. And pad it with zeroes up to 24 bytes ({ 1 } -> { 0, 0, 0, ..., 0, 1 })
  2. And then calling GenerateAddress of WASM-wrapper,

    1. which calls https://github.com/spacemeshos/address/blob/main/address.go#L146-L153
    2. and then converts the Address type to String, so it calls the same code as you have posted above ;)

    Btw, this spacemeshos/address repo should be reused in go-spacemesh as library:
    Use spacemeshos/address dependency go-spacemesh#3984

@joe-u410
Copy link
Author

joe-u410 commented Jun 6, 2023

Is there any documentation for the purpose of calculating the principal, versus just bech encoding the pubkey like most other networks using Bech32? What is the point behind the zero padding?

@lrettig
Copy link
Member

lrettig commented Jun 6, 2023

the purpose of calculating the principal, versus just bech encoding the pubkey like most other networks

This has to do with our model of account unification (account abstraction). See spacemeshos/SMIPS#49. We need to do a better job of documenting this. In short, we don't use simple keypair-based accounts (externally owned accounts) in Spacemesh. All accounts are smart contract (template)-based accounts. So we need to convert the pubkey for the user's keypair into the corresponding "smart wallet" account address.

What is the point behind the zero padding?

We're reserving 4 bytes in the address for future use. See spacemeshos/pm#106.

@pigmej pigmej added the milestone/genesis Should be done prior to genesis label Jun 19, 2023
@brusherru
Copy link
Member

What is the point behind the zero padding?

@joe-u410 if you meant why I pad it with zeroes up to 24 bytes:

Because otherwise, the spacemeshos/address library will return a zero-ish address and error: https://github.com/spacemeshos/address/blob/main/address.go#L78-L81

@brusherru
Copy link
Member

There is no problem. So I'm closing the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
milestone/genesis Should be done prior to genesis
Projects
Archived in project
Development

No branches or pull requests

5 participants