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 fix for issue 479 as a transformer #3160

Merged
merged 1 commit into from
Sep 1, 2024

Conversation

lorisleiva
Copy link
Collaborator

@lorisleiva lorisleiva commented Aug 26, 2024

This PR refactors the internal fix for issue 479 as a request transformer called getFixForIssue479RequestTransformer. this is part of a series of effort to extract all transformers into their own helper so the default transformer can be a simple composition of all of them.

Copy link

changeset-bot bot commented Aug 26, 2024

⚠️ No Changeset found

Latest commit: 96a54e8

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.

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

Copy link
Collaborator

@buffalojoec buffalojoec left a comment

Choose a reason for hiding this comment

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

Do we need this transformer anymore? The fix was backported to 1.18, which is the version the RPC is running on mainnet-beta.

@lorisleiva lorisleiva force-pushed the loris/add-get-default-commitment-request-transformer branch from 655a250 to 891c8c0 Compare August 30, 2024 16:38
@lorisleiva
Copy link
Collaborator Author

@buffalojoec Oh I didn't know this. In this case it's probably safe to remove and I'll add a PR to this stack to do this. Cc @steveluscher to double-check it's okay to remove?

@@ -27,29 +27,34 @@ export function getDefaultRequestTransformerForSolanaRpc(config?: RequestTransfo
const initialState = {
keyPath: [],
};
const patchedRequest = { methodName, params: traverse(rawParams, initialState) as T };
const patchedRequest = { methodName, params: traverse(rawParams, initialState) };
return pipe(
patchedRequest,
getDefaultCommitmentRequestTransformer({
defaultCommitment,
optionsObjectPositionByMethod: OPTIONS_OBJECT_POSITION_BY_METHOD,
}),
// FIXME Remove when https://github.com/anza-xyz/agave/pull/483 is deployed.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Side note: pretty sure we can delete this whole entire thing, because this was released in 1.18.13 and >=1.18.15 is the network now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

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 Sep 1, 2024

Merge activity

  • Sep 1, 2:06 PM EDT: @lorisleiva started a stack merge that includes this pull request via Graphite.
  • Sep 1, 2:30 PM EDT: Graphite rebased this pull request as part of a merge.
  • Sep 1, 2:31 PM EDT: @lorisleiva merged this pull request with Graphite.

@lorisleiva lorisleiva changed the base branch from loris/add-get-default-commitment-request-transformer to graphite-base/3160 September 1, 2024 18:26
@lorisleiva lorisleiva changed the base branch from graphite-base/3160 to master September 1, 2024 18:28
@lorisleiva lorisleiva merged commit fcfaec9 into master Sep 1, 2024
7 checks passed
@lorisleiva lorisleiva deleted the loris/refactor-fix-479 branch September 1, 2024 18:31
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 Sep 16, 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