Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

Encode to ReadonlyUint8Array in codecs #2392

Merged
merged 1 commit into from
Apr 2, 2024
Merged

Encode to ReadonlyUint8Array in codecs #2392

merged 1 commit into from
Apr 2, 2024

Conversation

mcintyre94
Copy link
Contributor

@mcintyre94 mcintyre94 commented Mar 28, 2024 •
edited by steveluscher
Loading

This PR refactors codecs to always return ReadonlyUint8Array instead of Uint8Array

I'm suspicious how straightforward this one was!

Addresses #2362.

@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Mar 28, 2024 •
edited
Loading

⚠️ No Changeset found

Latest commit: 1fcd4a9

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@mcintyre94 Graphite App
Copy link
Contributor Author

mcintyre94 commented Mar 28, 2024 •
edited
Loading

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

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

@mcintyre94 mcintyre94 mentioned this pull request Mar 28, 2024
Merged
@mcintyre94 mcintyre94 changed the title refactor(experimental): encode to ReadonlyUint8Array in codecs Encode to ReadonlyUint8Array in codecs Mar 28, 2024
@mcintyre94 mcintyre94 marked this pull request as ready for review March 28, 2024 19:25
@mcintyre94 mcintyre94 force-pushed the codecs-encode-readonly branch 2 times, most recently from 615f470 to 28fe2a7 Compare March 28, 2024 20:19
@mcintyre94 mcintyre94 requested a review from buffalojoec March 28, 2024 20:20
This was referenced Mar 28, 2024
Merged
Merged
Merged
lorisleiva
lorisleiva approved these changes Mar 29, 2024
View reviewed changes
Copy link
Contributor

@lorisleiva lorisleiva left a comment

Choose a reason for hiding this comment

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

Love it!

lorisleiva
lorisleiva reviewed Mar 29, 2024
View reviewed changes
packages/transactions/src/signatures.ts
Promise.all([getAddressFromPublicKey(keyPair.publicKey), signBytes(keyPair.privateKey, wireMessageBytes)]),
Promise.all([
getAddressFromPublicKey(keyPair.publicKey),
signBytes(keyPair.privateKey, wireMessageBytes as Uint8Array),
Copy link
Contributor

Choose a reason for hiding this comment

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

You should be able to remove that cast on the next PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And also kill this entire file 1,000,000 PRs into the future :D

steveluscher
steveluscher approved these changes Apr 1, 2024
View reviewed changes
Copy link
Contributor

@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

packages/library-legacy-sham/src/transaction.ts
@@ -55,7 +55,7 @@ export class Transaction {
'us know: https://github.com/solana-labs/solana-web3.js/issues/new/choose',
);
}
const byteArray = getTransactionEncoder().encode(this.#tx as CompilableTransaction);
const byteArray = getTransactionEncoder().encode(this.#tx as CompilableTransaction) as Uint8Array;
Copy link
Contributor

Choose a reason for hiding this comment

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

LOL.

Also, I should really delete this after you land this so that you never have to spend time on it again.

@mcintyre94 mcintyre94 force-pushed the codecs-decode-readonly branch 2 times, most recently from f6d9e8e to 6d5b8ef Compare April 2, 2024 15:04
Base automatically changed from codecs-decode-readonly to master April 2, 2024 16:09
@mcintyre94 mcintyre94 force-pushed the codecs-encode-readonly branch from 28fe2a7 to 1fcd4a9 Compare April 2, 2024 16:10
@mcintyre94 mcintyre94 merged commit 4326da8 into master Apr 2, 2024
8 checks passed
@mcintyre94 mcintyre94 deleted the codecs-encode-readonly branch April 2, 2024 16:18
@github-actions GitHub Actions
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 📦🚀

@github-actions GitHub Actions
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.
Reviewers

@lorisleiva lorisleiva lorisleiva approved these changes

@steveluscher steveluscher steveluscher approved these changes

@buffalojoec buffalojoec Awaiting requested review from buffalojoec

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mcintyre94 @steveluscher @lorisleiva