search

spacemeshos / go-spacemesh

Go Implementation of the Spacemesh protocol full node. 💾⏰💪
https://spacemesh.io
MIT License
748 stars 211 forks source link

genvm.sdk.multisig.SelfSpawn takes wrong key type #6061

Open lrettig opened 3 months ago

lrettig commented 3 months ago

Description

This method takes pubs public keys as type ed25519.PublicKey:

https://github.com/spacemeshos/go-spacemesh/blob/1be211be3f9b15b6b6a47a9f965a3a519439eea2/genvm/sdk/multisig/tx.go#L73-L89

However, internally it immediately converts these keys to core.PublicKey. What's more, the spawn arguments for the same type require an array of core.PublicKey:

https://github.com/spacemeshos/go-spacemesh/blob/1be211be3f9b15b6b6a47a9f965a3a519439eea2/genvm/templates/multisig/types.go#L13-L17

Let's unify these types. SelfSpawn should probably take core.PublicKey.

This issue appears in commit hash: 1be211be3f9b15b6b6a47a9f965a3a519439eea2

dshulyak commented 3 months ago

note that core.PublicKey is a 32 byte array, the reason why it is a an array is to avoid length prefixing every public key when encoding spawn arguments. obviously the same is true for decoding. this conversion is intentional.

ed25519.PublicKey is a slice, that doesn't do copy when passed around, and also every crypto library exposes public key as a slice, so it won't be convenient to do a cast to *[32]byte earlier.

i doubt that "unification" of types makes sense, it is better to close this issue

lrettig commented 3 months ago

This was inspired by https://github.com/spacemeshos/smcli/pull/115 where I was confused and frustrated by the need to pass multiple key types in different places

dshulyak commented 3 months ago

is it because you had to use spawn arguments struct directly? and in addition to that used "sdk" API? i looked at functions in sdk module and it doesn't accept core.PublicKey anywhere