-
Notifications
You must be signed in to change notification settings - Fork 92
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
server/eth: Allow geth connections using http and ws #2047
Conversation
dex/networks/eth/common.go
Outdated
@@ -55,7 +55,7 @@ func CheckAPIModules(c *rpc.Client, endpoint string, log dex.Logger, reqModules | |||
for v := range reqModulesMap { | |||
reqs = append(reqs, v) | |||
} | |||
return fmt.Errorf("needed apis not present: %v", strings.Join(reqs, " ")) | |||
return fmt.Errorf("needed apis not present: %v. some APIs may only be available over websocket and not http.", strings.Join(reqs, " ")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious if there was a specific case where you found ws had it but not http. I assume that's solvable with the --http.api=
flag, unless you were referring to a third-party RPC provider.
I'm afraid we may have to go with the "dumbed down" balance check #2020 because I haven't seen any RPC providers (other than geth itself) that provide txpool
. e.g. https://docs.alchemy.com/reference/ethereum-api-faq#what-methods-does-alchemy-support-for-the-ethereum-api, https://docs.blastapi.io/blast-documentation/apis-documentation/ethereum
Or we could at least fall back to that method when txpool is not available. Not needed for this PR, but have been thinking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah you're right, I just had to add -http.api.
Good stuff. dex-test:
|
In order to test the connection using ws, in the eth backend's rpc_harness_test, the delta node (the only one that has enabled http and ws) is used to send funds instead of the alpha node. The rpc_harness_test had pre-existing issues: - panicing due to registering the test token twice - incorrectly checking the difference in token balance. These issues are now fixed.
In order to test the connection using ws, in the eth backend's
rpc_harness_test, the delta node (the only one that has enabled
http and ws) is used to send funds instead of the alpha node.
The rpc_harness_test had pre-existing issues:
These issues are now fixed.
Closes #2026