Skip to content

Conversation

@rickyrombo
Copy link
Contributor

@rickyrombo rickyrombo commented Aug 14, 2024

Description

Before this change, all three of the program clients inherited from a BaseSolanaProgramClient which managed the RPC connection and had helpers for various tasks around building, sending, and confirming transactions. This PR breaks the inheritance model in favor of composition.

After this change, the structure of Solana in the SDK looks something like:

  • RewardManagerClient, ClaimableTokensClient, PaymentRouterClient depend on SolanaClient for interacting with RPC connection, get fee payer etc
  • SolanaClient depends on SolanaWalletAdapter to find out the fee payer and send transactions
  • SolanaRelayWalletAdapter implements SolanaWalletAdapter using SolanaRelay, building a normal WalletAdapter out of the relay service
  • SolanaRelay is the API interface for the Solana relay service endpoints

The pros of this:

  • allows us to not have to arbitrarily decide where to pull the RPC connection from
  • makes the testing surface for each client smaller
  • more clearly defines responsibilities

The cons:

  • now the API level needs a SolanaClient as well to build transactions
  • now have to think about what to use to call sendTransaction (maybe this shouldn't exist on the SolanaClient at all, but at the Program client level?)
  • SO many layers 🥴

How Has This Been Tested?

Tested sending a tip, will also test other flows though.

@changeset-bot
Copy link

changeset-bot bot commented Aug 14, 2024

🦋 Changeset detected

Latest commit: c12fdec

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

This PR includes changesets to release 2 packages
Name Type
@audius/sdk Major
@audius/sp-actions 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

@audius-infra
Copy link
Collaborator

Preview this change https://demo.audius.co/mjp-compose-solana-client

Copy link
Member

@raymondjacobson raymondjacobson left a comment

Choose a reason for hiding this comment

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

mucho better

}
this.connection = new Connection(
// Only supports one RPC endpoint right now
configWithDefaults.rpcEndpoints[0],
Copy link
Member

Choose a reason for hiding this comment

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

is this gonna give us a surprise problem? can we at least do a .random?

})
transaction.sign([sender])
const signature = await sdk.services.paymentRouterClient.sendTransaction(
const signature = await sdk.services.claimableTokensClient.sendTransaction(
Copy link
Contributor

Choose a reason for hiding this comment

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

wait why is this one not solanaClient?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yeah. It should be! Good catch

Copy link
Contributor

@dharit-tan dharit-tan left a comment

Choose a reason for hiding this comment

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

yeah this seems better! they should all be sharing the same connection/rpc/feepayer, and we shouldn't have to configure each of them with the same values copy/pasted.

@rickyrombo rickyrombo enabled auto-merge (squash) August 15, 2024 00:01
@rickyrombo rickyrombo disabled auto-merge August 15, 2024 00:01
@rickyrombo rickyrombo merged commit 363af71 into main Aug 15, 2024
@rickyrombo rickyrombo deleted the mjp-compose-solana-client branch August 15, 2024 00:01
schottra added a commit that referenced this pull request Aug 15, 2024
…ts-round-3

* origin/main: (23 commits)
  Audius Protocol v0.6.169
  wrong core defaults (#9452)
  Don't decrypt plaintext messages (#9448)
  [QA-1495] Fix 404 for users (#9451)
  [PAY-3227] Unfurl tracks in composer (#9434)
  Compose Solana Client in SDK (#9441)
  Create ChatBlast from web (#9447)
  [PAY-3341] Add is_plaintext field for chat message (#9442)
  use const for constants (#9446)
  core default values (#9445)
  Rename Targeted Message to Chat Blast (#9443)
  [C-4964] Comment header changes (#9444)
  [PAY-3310] Chat blast function in sdk (#9436)
  Move comments context to common (#9440)
  [C-4910] Comments replys & deletes fixes (#9437)
  Audius Protocol v0.6.168
  [PAY-3106][PAY-2914] Use SDK to withdraw USDC (#9419)
  Add social signup feature flag (#9439)
  use comet repo and modd for hotreloading (#9431)
  Fix duplicate error name in sentry (#9430)
  ...
audius-infra pushed a commit that referenced this pull request Aug 17, 2024
[d712b07] [C-4939] Add comment action menu popup (#9472) JD Francis
[641c9c1] [C-4940] Add comment badges (desktop web) (#9475) JD Francis
[aca7b5c] [PAY-3315][PAY-3316][PAY-3323] Staking bridge (#9408) Saliou Diallo
[bdc139a] [C-4828] Add timestamp features to comments (#9455) JD Francis
[3457aeb] [QA-1504] Fix payout wallet set default (#9467) Raymond Jacobson
[8c127d0] Add comment replies pagination (#9435) Isaac Solo
[c5e250e] [C-4929] Add SendIcon component to harmony and harmony native (#9422) Kyle Shanks
[a17b30e] Hook up client blast params to sdk (#9466) Reed
[11bf6e3] Don't decrypt plaintext chat.last_message (#9470) Reed
[bcaebb0] fix TS error (#9468) JD Francis
[bb802b7] Optimistic add chat blasts (#9463) Reed
[b5de040] Fix bad claimable challenge state due to nullable max_steps (#9461) Randy Schott
[d4c34c3] Fix comment delete in audius-query (#9462) JD Francis
[95380d8] [C-4800] Scaffolding for mobile comments (#9449) Sebastian Klingler
[fbb4256] [PAY-2917] Migrate user endpoints to SDK, round 3 (#9432) Randy Schott
[2c1679a] [PAY-3307] Upgrade chat blasts to chats in sdk (V2) (#9453) Reed
[279a9b2] More explicit audience columns for chat blast. (#9454) Steve Perkins
[acb857e] Fix bug in profile page 404 (#9456) Raymond Jacobson
[c382e82] Don't decrypt plaintext messages (#9448) Reed
[172f172] [QA-1495] Fix 404 for users (#9451) Raymond Jacobson
[82ab743] [PAY-3227] Unfurl tracks in composer (#9434) Raymond Jacobson
[363af71] Compose Solana Client in SDK (#9441) Marcus Pasell
[83260f6] Create ChatBlast from web (#9447) Andrew Mendelsohn
[19e3bdf] [PAY-3341] Add is_plaintext field for chat message (#9442) Steve Perkins
[e25c679] Rename Targeted Message to Chat Blast (#9443) Reed
[fc0604c] [C-4964] Comment header changes (#9444) JD Francis
[f9e4525] [PAY-3310] Chat blast function in sdk (#9436) Reed
[910b655] Move comments context to common (#9440) JD Francis
[bb5eaa1] [C-4910] Comments replys & deletes fixes (#9437) JD Francis
[1072f5a] [PAY-3106][PAY-2914] Use SDK to withdraw USDC (#9419) Marcus Pasell
[1b7bd47] Add social signup feature flag (#9439) Sebastian Klingler
[4130f42] Fix duplicate error name in sentry (#9430) Marcus Pasell
[54a5a78] [PAY-3300] Chat blast: lazy create thread (#9424) Steve Perkins
[ca71d76] [C-4910] Audius-query-ify comments state management (#9423) JD Francis
[20a55e9] [PAY-2917][QA-1466] Migrate more users endpoints (#9308) Randy Schott
[ab38869] [QA-1434] Fix hanging uploads due to errors on mobile (#9420) Randy Schott
[f6ab71c] Add buy button metrics (#9428) Isaac Solo
[06ee8a1] Fix pagination args (#9425) Isaac Solo
[b91437a] [QA-1501] Fix mobile search input (#9426) Sebastian Klingler
[a3c15fd] [C-4932] Add CommentBody component to harmony and harmony-native (#9415) Kyle Shanks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants