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

token: Reconsider unsafe usage for instruction pack/unpack #315

Closed
mvines opened this issue Aug 25, 2020 · 4 comments
Closed

token: Reconsider unsafe usage for instruction pack/unpack #315

mvines opened this issue Aug 25, 2020 · 4 comments
Assignees
Milestone

Comments

@mvines
Copy link
Member

mvines commented Aug 25, 2020

The use of unsafe typecasting in instruction pack/unpack significantly reduces the instruction count over alternatives but is also scary to look at and is more likely it contain bugs (or have bugs accidentally introduced). Further this is not a general pattern we want to see in all Solana contracts.

Consider alternatives that either make the unsafe go away entire, or pushes the unsafe into macros or generic libraries where the possibility of human error is reduced.

@mvines mvines added this to the Token 2 milestone Aug 25, 2020
@mvines
Copy link
Member Author

mvines commented Aug 26, 2020

https://crates.io/crates/byteorder has been used by some

@mvines
Copy link
Member Author

mvines commented Aug 27, 2020

Also @jackcmay raised the point that since the instruction data is now coming in aligned in the new BPF loader that using byteorder may just be as quick as unsafe casts/copies

@jackcmay
Copy link
Contributor

Byte order will not be as quick and most likely significantly slower due to copies both into and back out to account data vs cast and use in place

@mvines
Copy link
Member Author

mvines commented Aug 28, 2020

Fixed by #349

@mvines mvines closed this as completed Aug 28, 2020
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

No branches or pull requests

2 participants