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

feat: Tor/SOCKS5 proxy support: add optional proxyInfo property and use it (via the socks5_proxy package) if not null. #1378

Closed
wants to merge 1 commit into from

Conversation

sneurlax
Copy link

Closes #1377.

Adds an optional proxyInfo property to a Solana RpcClient. If it's null (as old clients will pass it by default), it changes no behavior. If it's not null, it uses the provided proxyInfo to proxy requests. Pass proxyInfo as a Map<String, dynamic> {String host, int port}.

Dart 3 supports Records, in which case proxyInfo could be ({String host, int port})? proxyInfo.

Changes

  • Add the socks5_proxy package. This is widely used and used by Stack Wallet.

Related issues

Fixes #1377

Checklist

  • PR is ready for review (if not, it should be a draft).
  • PR title follows Conventional Commits guidelines.
  • Screenshots/video not applicable.
  • Tests unaffected.
  • Self-review done.

Pass proxyInfo as a Map<String, dynamic> {String host, int port}.
Copy link
Collaborator

@ookami-kb ookami-kb left a comment

Choose a reason for hiding this comment

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

I don't think that adding another 3rd-party library for such a specific use case makes sense.

I would rather suggest making JsonRpcClient._postRequest method protected, and adding

factory RpcClient.withCustomClient(JsonRpcClient client) {}

In that case, you can implement your custom behavior by extending JsonRpcClient class and passing it to the RpcClient factory.

@romeo4934 romeo4934 closed this Apr 22, 2024
@sneurlax
Copy link
Author

I am currently testing the changes @ookami-kb requested, should I just open a new PR @antoineherzog?

@ookami-kb
Copy link
Collaborator

@sneurlax feel free to either reopen this one or create a new one - whatever is more convenient for you.

@sneurlax
Copy link
Author

sneurlax commented Apr 23, 2024

There have been a series of issues implementing it as recommended. The only solution I've got working as requested is very long because it has to implement overrides for every method in JsonRpcClient. I really tried to minimize the diff and avoid these overrides, but the most elegant solution requires use of dart:mirrors in order to get the expected return types of methods, which is not possible because out app is Dart 3, not the outdated Dart 2 which this package uses. Dynamic return types and delegating methods in the custom RPC client using noSuchMethod might be a shorter solution, but I could not ultimately get that approach working in Dart 3.

I would almost be content to say "Stack Wallet is the only Dart/Flutter wallet to support Tor for Solana" rather than continue working on minimizing the changes necessary to get it working as requested. We're currently simply using our cypherstack/espresso-cash-public fork to accomplish this currently using the code originally submitted as-is--not using the changes as requested (although I tested them working). I don't like using an unmaintained fork, though, hence my attempt at fulfilling the requested code change.

@ookami-kb can you consider simply accepting the original patch? socks5_proxy is one of the most widely used SOCKS packages and is Dart 2 and 3 compatible. It would minimize the diff required for your Dart/Flutter users to have any option for proxy use. Otherwise, the changes required to get it working are as shown on my subsequent PR (I can't reopen this one).

@sneurlax
Copy link
Author

update: as I have been testing more functions, I have been finding more issues. I don't have enough Solana to test all functions and I really, really don't like this solution anyways. See 1,000+ lines of biolerplate at: https://github.com/cypherstack/espresso-cash-public/blob/test/packages/solana/lib/src/rpc/custom_client.dart

@ookami-kb, what am I doing wrong? You made this approach sound easy and simple, but it has been anything but easy and simple for me over here.

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.

Tor/SOCKS5 proxy support for Dart Solana RpcClient
3 participants