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): add prefixCodec to @solana/codecs-core #2397

Merged
merged 1 commit into from
Apr 2, 2024

Conversation

lorisleiva
Copy link
Collaborator

@lorisleiva lorisleiva commented Mar 29, 2024

This PR adds a new prefixCodec primitive to @solana/codecs-core. This primitives accepts any codec and prefixes its size using a NumberCodec provided as a second argument.

For instance:

const codec = prefixCodec(getBase58Codec(), getU32Codec());

codec.encode('hello world');
// 0x0b00000068656c6c6f20776f726c64
//   |       └-- Our encoded base-58 string.
//   └-- Our encoded u32 size prefix.

This PR also updates the getStringCodec and getBytesCodec as they can now use the prefixCodec primitive to simplify their internal logic.

P.S.: Now that getStringCodec and getBytesCodec are simple aggregator of fixCodec and prefixCodec, we should consider removing them or adjusting them further. (Pros: no default values => better tree-shakeability, Cons: slightly less friendly API?).

// With getStringCodec.
getStringCodec();
getStringCodec({ encoding: getBase58Codec() });
getStringCodec({ size: 32 });
getStringCodec({ size: getU16Codec() });
getStringCodec({ size: "variable" });

// Without getStringCodec.
prefixCodec(getUtf8Codec(), getU32Codec());
prefixCodec(getBase58Codec(), getU32Codec());
fixCodec(getUtf8Codec(), 32);
prefixCodec(getUtf8Codec(), getU16Codec());
getUtf8Codec();

Copy link

changeset-bot bot commented Mar 29, 2024

🦋 Changeset detected

Latest commit: 2a1b158

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

This PR includes changesets to release 35 packages
Name Type
@solana/codecs-data-structures Patch
@solana/codecs-strings Patch
@solana/codecs-core Patch
@solana/codecs Patch
@solana/transactions Patch
@solana/accounts Patch
@solana/addresses Patch
@solana/keys Patch
@solana/rpc-api Patch
@solana/rpc-graphql Patch
@solana/rpc-types Patch
@solana/transaction-confirmation Patch
@solana/codecs-numbers Patch
@solana/options Patch
@solana/web3.js-experimental Patch
@solana/sysvars Patch
@solana/compat Patch
@solana/web3.js-legacy-sham Patch
@solana/programs Patch
@solana/rpc-subscriptions-api Patch
@solana/signers Patch
@solana/instructions Patch
@solana/rpc-parsed-types Patch
@solana/rpc Patch
@solana/rpc-subscriptions Patch
@solana/rpc-transformers Patch
@solana/assertions Patch
@solana/errors Patch
@solana/functional Patch
@solana/rpc-spec-types Patch
@solana/rpc-spec Patch
@solana/rpc-subscriptions-spec Patch
@solana/rpc-subscriptions-transport-websocket Patch
@solana/rpc-transport-http 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

lorisleiva commented Mar 29, 2024

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.

tl;dr unless I missed it, I think you need something to prevent the actual size of the data from being larger than that which you can represent using the length prefix. We either need to throw when that's the case, or truncate the data to the actual length that will eventually be decoded.

packages/codecs-core/README.md Show resolved Hide resolved
packages/codecs-core/README.md Outdated Show resolved Hide resolved
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.

Added via Giphy

Copy link
Collaborator Author

lorisleiva commented Apr 2, 2024

Merge activity

  • Apr 2, 6:48 PM EDT: @lorisleiva started a stack merge that includes this pull request via Graphite.
  • Apr 2, 6:53 PM EDT: Graphite rebased this pull request as part of a merge.
  • Apr 2, 6:54 PM EDT: @lorisleiva merged this pull request with Graphite.

Base automatically changed from loris/small-codec-refactor to master April 2, 2024 22:52
@lorisleiva lorisleiva merged commit a548de2 into master Apr 2, 2024
6 checks passed
@lorisleiva lorisleiva deleted the loris/prefix-codec branch April 2, 2024 22:54
lorisleiva added a commit that referenced this pull request Apr 2, 2024
…ze and prefixCodecSize (#2411)

This PR renames `fixCodec` to `fixCodecSize` and `prefixCodec` to `prefixCodecSize` [as discussed here](#2397 (comment)).

Note that I created a new changeset describing the change for `fixCodec` but not for `prefixCodec` as this is not yet merged and I thought it'd be better to update the changeset created by a previous PR of this stack.
Copy link
Contributor

github-actions bot commented Apr 3, 2024

🎉 This PR is included in version 1.91.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

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 Apr 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants