Swap spl-pod for solana-zero-copy + solana-nullable#1069
Conversation
grod220
force-pushed
the
spl-pod-swap
branch
6 times, most recently
from
d655ba0 to
b68f68c
Compare
| spl-token-metadata-interface = "1.0.0" | ||
| spl-token-group-interface = "0.7.2" |
There was a problem hiding this comment.
New versions support the new solana-zero-copy/nullable types
| [target.'cfg(all(target_arch = "wasm32", target_os = "unknown"))'.dependencies] | ||
| getrandom = { version = "0.2", features = ["js"] } |
There was a problem hiding this comment.
Without this, make build-wasm-interface fails.
the wasm*-unknown-unknown targets are not supported by default, you may need to enable the "js" feature
Turns out spl-pod previously activates getrandom/js transitively. So need to be explicit here now.
There was a problem hiding this comment.
This actually points at another issue I think -- currently spl-token-2022-interface is importing spl-confidential-transfer-proof-generation just for a conversion between error types.
I would instead just remove the error conversion and the dependency entirely in spl-token-2022-interface. The conversion is likely only needed by spl-token-client or spl-token-cli.
This can be done in a follow-up, however. To make it super clear though, can you add these lines to spl-token-2022-interface instead in the meantime?
What do you think @samkim-crypto ?
There was a problem hiding this comment.
Yes, I missed this. We should definitely remove spl-confidential-transfer-proof-generation out of the interface. I can take care of this in a follow-up PR.
| #[cfg_attr(feature = "serde", serde(with = "As::<Option<DisplayFromStr>>"))] | ||
| pub authority: MaybeNull<Address>, |
There was a problem hiding this comment.
the serde_with usage here is to preserve the base58 serde encoding that was built into OptionalNonZeroPubkey
| arrayref = "0.3.9" | ||
| bytemuck = { version = "1.25.0", features = ["derive"] } | ||
| num-derive = "0.4" | ||
| num-traits = "0.2" |
There was a problem hiding this comment.
While deps/features were being added, used cargo machete to find a collection of unused ones in this program and removed them
joncinque
left a comment
joncinque
left a comment
There was a problem hiding this comment.
Looks really great overall! Thanks for handling all this tedious work 🙏
Just a couple of points
| no-entrypoint = [] | ||
| test-sbf = [] | ||
| serde-traits = ["dep:serde", "dep:serde_with", "dep:base64", "spl-pod/serde-traits", "spl-token-2022-interface/serde"] | ||
| serde-traits = ["spl-token-2022-interface/serde"] |
There was a problem hiding this comment.
For a follow-up, how about renaming this feature to serde?
There was a problem hiding this comment.
Yep, can follow up with this
| [target.'cfg(all(target_arch = "wasm32", target_os = "unknown"))'.dependencies] | ||
| getrandom = { version = "0.2", features = ["js"] } |
There was a problem hiding this comment.
This actually points at another issue I think -- currently spl-token-2022-interface is importing spl-confidential-transfer-proof-generation just for a conversion between error types.
I would instead just remove the error conversion and the dependency entirely in spl-token-2022-interface. The conversion is likely only needed by spl-token-client or spl-token-cli.
This can be done in a follow-up, however. To make it super clear though, can you add these lines to spl-token-2022-interface instead in the meantime?
What do you think @samkim-crypto ?
joncinque
left a comment
joncinque
left a comment
There was a problem hiding this comment.
Looks good to me! Do get Sam's approval though to be sure
samkim-crypto
left a comment
samkim-crypto
left a comment
There was a problem hiding this comment.
Looks good to me on my side also!
3 participants
rebased after #1090
This PR migrates
token-2022away fromspl-podand ontosolana-zero-copy+solana-nullable. On the road to making the library crates no-std.