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

minor features and improvements #13

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

Conversation

defactojob
Copy link
Contributor

No description provided.

@@ -17,6 +18,21 @@ const (

type PublicKey [PublicKeyLength]byte

func (pk *PublicKey) UnmarshalJSON(b []byte) error {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you add test for this ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'll handle this a bit later

@@ -23,3 +29,19 @@ func AccountFromPrivateKeyBytes(privateKey []byte) Account {
PrivateKey: sk,
}
}

func (a *Account) UnmarshalText(b []byte) error {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I more prefer AccountFromHex. WDYT

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UnmarshalText is an interface from the standard library, to support some parsing libraries.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh! Ok. Would you add some test for it?

Comment on lines +16 to +22
// MintAuthorityOption uint32
MintAuthority *common.PublicKey
Supply uint64
Decimals uint8
IsInitialized bool
// FreezeAuthorityOption uint32
FreezeAuthority *common.PublicKey
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can remove the // MintAuthorityOption uint32 and // FreezeAuthorityOption uint32.

Comment on lines +25 to +61
const (
mintMintAuthorityOptionOffset = 0
mintMintAuthorityOffset = mintMintAuthorityOptionOffset + 4
mintSupplyOffset = mintMintAuthorityOffset + 32
mintDecimalsOffset = mintSupplyOffset + 8
mintIsInitializedOffset = mintDecimalsOffset + 1
mintFreezeAuthorityOptionOffset = mintIsInitializedOffset + 1
mintFreezeAuthorityOffset = mintFreezeAuthorityOptionOffset + 4
)

func isSome(option []byte) bool {
return bytes.Equal(option, Some)
}

func MintAccountFromData(data []byte) (*MintAccount, error) {
if len(data) != MintAccountSize {
return nil, fmt.Errorf("mint account data length mismatch")
}

var mint MintAccount

mintAuthorityOption := data[0:4]
if isSome(mintAuthorityOption) {
key := common.PublicKeyFromBytes(data[mintMintAuthorityOffset : mintSupplyOffset+32])
mint.MintAuthority = &key
}

mint.Supply = binary.LittleEndian.Uint64(data[mintSupplyOffset : mintSupplyOffset+8])
mint.Decimals = uint8(data[mintDecimalsOffset])
mint.IsInitialized = data[mintIsInitializedOffset] == 1

if isSome(data[mintFreezeAuthorityOptionOffset:mintFreezeAuthorityOptionOffset+4]) {
key := common.PublicKeyFromBytes(data[mintFreezeAuthorityOffset : mintFreezeAuthorityOffset+32])
mint.FreezeAuthority = &key
}

return &mint, nil
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess maybe you want to make code more readable but I would like to use literal number like I decode token account and would you add a test for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how important is this to you to use literal number? using literal offsets is pretty unreadable to be honest.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to use literal number for some reasons.

  1. if someday I want to use get accountInfo to fetch data and using offset. the literal number is more intuitive to me. I don't need to recalculate it if I miss in the bytes array.
  2. I don't think there are someone will read this code, I want to make them as clear as possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if someday I want to use get accountInfo to fetch data and using offset.

if that's the case, wouldn't you want to export the offset constants rather than using literals?

@yihau yihau force-pushed the main branch 2 times, most recently from 1a94545 to 55b476d Compare February 28, 2022 18:22
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.

2 participants