Skip to content

refactor(solana): remove dummy ws connection in SolanaClient - #424

Open
swimthor wants to merge 1 commit into
masterfrom
test-if-need-dummy-connection
Open

swimthor wants to merge 1 commit into
masterfrom
test-if-need-dummy-connection

Conversation

@swimthor

@swimthor swimthor commented Oct 9, 2022

Copy link
Copy Markdown
Contributor

It passes the qa full test in https://exsphere.slack.com/archives/C02PVCS2PKQ/p1665410870060659?thread_ts=1665393074.911999&cid=C02PVCS2PKQ

To be honest I didn't see any actual code change in @solana/web3.js since #284 (comment). But I am align with Andrew this dummy ws is not needed because when we need the onSignature ws, it will create a new ws connection via https://github.com/solana-labs/solana-web3.js/blob/master/src/connection.ts#L5025-L5028

Notion ticket: https://www.notion.so/exsphere/Investigate-whether-we-still-need-a-dummy-websocket-connection-in-SolanaConnection-018975be2b1749a3b636644a07e2e5b9

Checklist

  • Github project and label have been assigned
  • Tests have been added or are unnecessary
  • Docs have been updated or are unnecessary
  • Preview deployment works (visit the Cloudflare preview URL)
  • Manual testing in #product-testing completed or unnecessary
  • New i18n strings do not contain any security vulnerabilities (e.g. should not allow translator to update email/url)
  • When fetching new translations from external sources, do a sanity check (e.g. get people who speak the language to check, shove all new translations into Google translate)

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Oct 9, 2022

Copy link
Copy Markdown

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 2820cd6
Status: ✅  Deploy successful!
Preview URL: https://05df999c.swim-ui.pages.dev
Branch Preview URL: https://test-if-need-dummy-connectio.swim-ui.pages.dev

View logs

@swimthor
swimthor force-pushed the test-if-need-dummy-connection branch from 2bd05a3 to 96f6c0c Compare October 10, 2022 10:24
@swimthor swimthor changed the title [DO NOT MERGE] Test if need dummy connection refactor(solana): remove dummy ws connection in SolanaClient Oct 10, 2022
@swimthor
swimthor requested review from swimdrew and wormat October 10, 2022 14:26
wormat
wormat previously approved these changes Oct 10, 2022

@wormat wormat left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

OK let's give this another go but ask the testers to try again on swim.io once it's merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants