-
Notifications
You must be signed in to change notification settings - Fork 54
Add solana-address
crate
#111
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
base: master
Are you sure you want to change the base?
Conversation
I do not have final thoughts on this yet but one benefit of the wrapper type is we can optimise them more than a plain arrays, for example changing the Hash impl: #96 |
Yeah, definitely there are trade-offs. But perhaps it is easier to go from an "alias" type to a "wrapper" when needed, like the case for the |
pub struct Pubkey(pub(crate) [u8; 32]); | ||
pub struct Pubkey(pub(crate) Address); | ||
|
||
impl Deref for Pubkey { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's the motivation for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could use Pubkey
references transparently in helpers that expect an &Address
:
fn expect_address(address: &Address) {
assert_eq!(address.len(), PUBKEY_BYTES);
}
let address: Address = [1u8; 32];
let pubkey: Pubkey = address.into();
expect_address(&address);
expect_address(&pubkey);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could also do
fn expect_address(address: impl AsRef<Address>) {
let as_ref: &Address = address.as_ref();
assert_eq!(as_ref.len(), PUBKEY_BYTES);
}
This would potentially accept types we don't really want to accept like Hash
, but it's more obvious behaviour
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that works as well – I don't have a strong preference. There are a couple of places that we will need to add a type annotation since Pubkey
already implements impl AsRef<[u8]> for Pubkey
, but that is not too bad.
Problem
While a Solana address is represented by a
[u8; 32]
array, addresses are currently represented by a "wrapper"Pubkey
type over the[u8; 32]
array.It is very common to use the
Pubkey
type in user-defined structs representing account data, which eventually are serialized. The drawback of using aPubkey
in this context is that aPubkey
needs to implement additional traits to support all the different serialization frameworks. For example, to use thezerocopy
crate to interact with account data, its traits need to be implemented on the SDK'sPubkey
type; the same is true for any other/new serialization framework.Solution
Define a lightweight
Address
type to represent Solana addresses:Since an
Address
is simply an alias to a[u8; 32]
array, most (if not all) serialization frameworks include support for byte arrays.The syscalls to derive program addresses are also moved to this crate, since they operate over a
[u8; 32]
array. Thesolana-pubkey
crate provide wrappers around the provided helpers to add the implementation for targets other than "solana".The
Address
type can be used to replace Pinocchio'sPubkey
type (see anza-xyz/pinocchio#118).Note
This PR depends on #12 so the failure of the "crate dependencies for publishing" is expected.
cc: @kevinheavey