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

refactor(experimental): consolidate Option-like codecs #2715

Merged
merged 1 commit into from
May 15, 2024

Conversation

lorisleiva
Copy link
Collaborator

@lorisleiva lorisleiva commented May 14, 2024

This PR consolidates the getNullableCodec and getOptionCodec with their Zeroable counterparts and adds more configurations.

Namely, the prefix option can now be set to null and the fixed option was replaced with the noneValue option which can be set to "zeroes" for Zeroable codecs or a custom byte array for custom representations of none values. This means the getZeroableNullableCodec and getZeroableOptionCodec functions were removed in favor of the new options.

// Before.
getZeroableNullableCodec(getU16Codec());

// After.
getNullableCodec(getU16Codec(), { noneValue: 'zeroes', prefix: null });

As a result, this PR makes it possible to create nullable codecs that have no prefix nor noneValue. In this case, the existence of the nullable item is indicated by the presence of any remaining bytes left to decode.

const codec = getNullableCodec(getU16Codec(), { prefix: null });
codec.encode(42); // 0x2a00
codec.encode(null); // Encodes nothing.
codec.decode(new Uint8Array([42, 0])); // 42
codec.decode(new Uint8Array([])); // null

Also note that it is now possible for custom noneValue byte arrays to be of any length — previously, it had to match the fixed-size of the nullable item. This means the SOLANA_ERROR__CODECS__EXPECTED_ZERO_VALUE_TO_MATCH_ITEM_FIXED_SIZE error is no longer used. I did not remove it because it is illegal but thought I needed to mention that here.

Here is a recap of all supported scenarios, using a u16 codec as an example:

encode(42) / encode(null) No noneValue (default) noneValue: "zeroes" Custom noneValue (0xff)
u8 prefix (default) 0x012a00 / 0x00 0x012a00 / 0x000000 0x012a00 / 0x00ff
Custom prefix (u16) 0x01002a00 / 0x0000 0x01002a00 / 0x00000000 0x01002a00 / 0x0000ff
No prefix 0x2a00 / 0x 0x2a00 / 0x0000 0x2a00 / 0xff

Reciprocal changes were made with getOptionCodec.

Why?

As I needed the getNullableCodec(x, { prefix: null }) variant, I could either add yet another Option-like codec pair such as getRemainderNullableCodec and getRemainderOptionCodec OR I could realise that actually, this is a simple 2x3 configuration matrix and refactor the getNullableCodec and getOptionCodec accordingly. I opted for the latter.

Copy link

changeset-bot bot commented May 14, 2024

🦋 Changeset detected

Latest commit: d20edaf

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 36 packages
Name Type
@solana/codecs-data-structures Patch
@solana/options Patch
@solana/codecs Patch
@solana/transaction-messages Patch
@solana/transactions Patch
@solana/web3.js-experimental Patch
@solana/sysvars Patch
@solana/programs Patch
@solana/rpc-api Patch
@solana/rpc-subscriptions-api Patch
@solana/signers Patch
@solana/transaction-confirmation Patch
@solana/compat Patch
@solana/rpc-graphql Patch
@solana/rpc Patch
@solana/rpc-subscriptions Patch
@solana/accounts Patch
@solana/addresses Patch
@solana/assertions Patch
@solana/codecs-core Patch
@solana/codecs-numbers Patch
@solana/codecs-strings Patch
@solana/errors Patch
@solana/fast-stable-stringify Patch
@solana/functional Patch
@solana/instructions Patch
@solana/keys Patch
@solana/rpc-parsed-types Patch
@solana/rpc-spec-types Patch
@solana/rpc-spec Patch
@solana/rpc-subscriptions-spec Patch
@solana/rpc-subscriptions-transport-websocket Patch
@solana/rpc-transformers Patch
@solana/rpc-transport-http Patch
@solana/rpc-types Patch
@solana/webcrypto-ed25519-polyfill Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Collaborator Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @lorisleiva and the rest of your teammates on Graphite Graphite

@lorisleiva lorisleiva marked this pull request as ready for review May 14, 2024 12:17
@lorisleiva lorisleiva requested review from steveluscher, mcintyre94 and buffalojoec and removed request for steveluscher and mcintyre94 May 14, 2024 12:18
Copy link
Collaborator

@mcintyre94 mcintyre94 left a comment

Choose a reason for hiding this comment

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

Nice, this looks like a cleaner API to me!

Copy link
Collaborator

@steveluscher steveluscher left a comment

Choose a reason for hiding this comment

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

didntread1

@lorisleiva lorisleiva merged commit 26dae19 into master May 15, 2024
8 checks passed
@lorisleiva lorisleiva deleted the loris/consolidate-option-like-codecs branch May 15, 2024 12:39
Copy link
Contributor

Because there has been no activity on this PR for 14 days since it was merged, it has been automatically locked. Please open a new issue if it requires a follow up.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants