eth: Add authenticated geth rpc capabilities. #1963

JoeGruffins · 2022-11-17T12:42:35Z

eth: Add authenticated geth rpc capabilities.

Add ability for client and server to connect over authenticated
websocket to a geth full node.

depends on #1733
closes #1956

JoeGruffins

I hope the typescript code doesn't hurt your eyes.

JoeGruffins · 2022-11-25T06:50:27Z

client/asset/eth/eth.go

@@ -73,7 +74,7 @@ const (
 	walletTypeGeth = "geth"
 	walletTypeRPC  = "rpc"

-	providersKey = "providers"
+	providersKey = "providersv1"


In this pr changing the type of value for the providers so currently saved settings will not parse correctly. Since eth is not being used by anyone yet probably this easy change is fine I think, but this would be a problem in production code.

JoeGruffins · 2022-11-25T06:52:34Z

client/asset/eth/nodeclient_harness_test.go

-//
-// TODO: Running these tests many times on simnet eventually results in all
-// transactions returning "unexpected error for test ok: exceeds block gas
-// limit". Find out why that is.


Have not seen this for the longest and never on testnet. Ok to remove I'm sure.

JoeGruffins · 2022-11-25T06:57:53Z

client/webserver/site/src/html/forms.tmpl

+  <div data-tmpl="repeatableInputOutter" class="w-100 repeatable">
+    <div data-tmpl="repeatableInput" class="w-100 repeatable">
+      <label class="form-label ps-1 mb-1 small"> <span class="ico-info"></span></label>
+      <div class="d-flex align-items-stretch justify-content-center">
+        <input type="text" class="form-control select flex-grow-1">
+        <div class="ico-plus fs14 p-1 ms-2 mt-1 pointer hoverbg" data-tmpl="add"></div>
+      </div>


This was the best answer I could come up with for the generic repeatables to have multiple fields. Probably others have a better solution, but this seems to work ok. The idea is that we can add new repeatables easier if we have the outter container, and we can keep them divided if there were more repeatable type options. Right now we only have the eth providers.

I may have a plan for this, but I'll follow up. Not really fleshed out yet.

JoeGruffins · 2022-11-25T07:03:07Z

dex/testing/eth/harness.sh

-export DELTA_HTTP_PORT="38556"
-export DELTA_WS_PORT="38557"


Delta is a light wallet, so I think we should not use them for testing at the moment. This was probably done on Delta because the wallet will allow the open http/ws ports because an account is not unlocked. The full nodes are unlocked for mining meaning they panic if you try to set these ports in the config. However, we can use the authed port even if the geth node has an unlocked account.

JoeGruffins · 2022-11-25T07:08:07Z

dex/testing/loadbot/go.mod

-	github.com/edsrzf/mmap-go v1.0.0 // indirect
-	github.com/ethereum/go-ethereum v1.10.25 // indirect
+	github.com/edsrzf/mmap-go v1.1.0 // indirect
+	github.com/ethereum/go-ethereum v1.10.24-0.20220902154041-90711efb0ab6 // indirect


v1.10.26 is actually latest, but it does not include the commit we need to use the webserver jwt auth. We need ethereum/go-ethereum@90711ef

I'm not sure why the go.mod wants v1.10.24, maybe it's the first common ancestor?

Looks like ethereum/go-ethereum@90711ef is not on the release/1.10 branch. The PR has the 1.11 milestone, so yeah, they branched the 1.10 maintenance branch after 1.10.24.

This could potentially be problematic if 1.11 is not very soon. We can't release anything that requires a dev branch. They look close, but it's unclear.

I'm a little surprised to see that we needed a new go-ethereum feature to use JTW auth since geth has supported JWT for a long time on the http interface (haven't they? or did the need just arise with authrpc.*?). Did they just now get to adding support to the client?

Did they just now get to adding support to the client?

Yes.

I gather the endpoint was created for connecting the beacon chain prism thing, but it connects over http, which is much simpler because you only need to send the auth token once in the initial header. With websocket you need to send it again on disconnects and there is no way to set that and use the provided client.

We could just use http for now and it would work with master.

martonp

Looks good, but haven't reviewed in detail.

One comment regarding the UI: I think the extra JWT field should be be optional because it will generally be unused. Maybe have a little button next to the url input to add a JWT. If someone has multiple RPCs and each of the JWT field is empty, it'll be slightly distracting. Not a big deal though.

client/webserver/site/src/js/forms.ts

client/asset/eth/eth.go

client/webserver/site/src/js/forms.ts

JoeGruffins · 2022-12-01T06:53:46Z

Marton fixes https://github.com/decred/dcrdex/compare/bf485b46e638bdbaddd31c2f161acf97ab255802..0c9788b330bb1b77c7acfff0c443e71ee657fd14

Maybe have a little button next to the url input to add a JWT.

hmmm

JoeGruffins · 2022-12-01T07:32:05Z

Fix tooltips https://github.com/decred/dcrdex/compare/0c9788b330bb1b77c7acfff0c443e71ee657fd14..d2a1986f3c734f9180dd813423f7df13c371bc41

For reference here is what the repeatable options currently look like:

go.mod

chappjc · 2022-12-13T18:57:16Z

dex/testing/loadbot/go.mod

-	github.com/edsrzf/mmap-go v1.0.0 // indirect
-	github.com/ethereum/go-ethereum v1.10.25 // indirect
+	github.com/edsrzf/mmap-go v1.1.0 // indirect
+	github.com/ethereum/go-ethereum v1.10.24-0.20220902154041-90711efb0ab6 // indirect


Looks like ethereum/go-ethereum@90711ef is not on the release/1.10 branch. The PR has the 1.11 milestone, so yeah, they branched the 1.10 maintenance branch after 1.10.24.

This could potentially be problematic if 1.11 is not very soon. We can't release anything that requires a dev branch. They look close, but it's unclear.

I'm a little surprised to see that we needed a new go-ethereum feature to use JTW auth since geth has supported JWT for a long time on the http interface (haven't they? or did the need just arise with authrpc.*?). Did they just now get to adding support to the client?

chappjc · 2022-12-13T19:05:15Z

server/asset/eth/config.go

+
+	cfg := new(config)
+
+	// NOTE: ipc only is deprecated. Remove this at some point.


Deprecated by geth? Or we are declaring this deprecated with dcrdex?

Us. After these changes we can connect over websocket or ipc.

After these changes we can connect over websocket or ipc.

Do we really want to remove support for IPC in the future though? I don't see a reason.

no, I just want to remove setting the ipc in the markets.json, and have it point to a config file like other coins, for one because noone is really using this yet and for another maybe we need more start up options in the future.

server/asset/eth/config.go

chappjc · 2022-12-13T19:23:02Z

dex/testing/eth/create-node.sh

 AuthPort = ${AUTHRPC_PORT}
+JWTSecret = "${NODE_DIR}/jwt.hex"


Now I'm confused about the API services geth offers. There's:

HTTPHost / HTTPPort

WSHost / WSPort

AuthAddr / AuthPort

The first two seem to be only "public" RPCs, by default, although there's a WSExposeAll to override that for ws (WSExposeAll exposes all API modules via the WebSocket RPC interface rather than just the public ones).

The "auth" ones specify the host:port "on which authenticated APIs are provided" but does that mean only authenticated APIs, and thus "public" ones aren't available on this port? I'm confused if "authenticated APIs" are defined as distinct from "public" RPCs (with no intersection), or do authenticated APIs include all the RPCs..

I would expect server should only require the "public API modules", so would server be able to work with geth started with just --http or --ws (not --authrpc?)

What modules are considered public and what are considered authenticated? Referring to:

// HTTPModules is a list of API modules to expose via the HTTP RPC interface. // If the module list is empty, all RPC API endpoints designated public will be // exposed. HTTPModules []string

// WSExposeAll exposes all API modules via the WebSocket RPC interface rather // than just the public ones. // // *WARNING* Only set this if the node is running in a trusted network, exposing // private APIs to untrusted users is a major security risk. WSExposeAll bool `toml:",omitempty"`

Where's the list of public vs non-public/private APIs? And which ones are served on AuthPort?

geth documentation is not great

I do know that if you try to start a node with an account unlocked and http on, it panics. Will look into it, but we do not want to set either --http or --ws.

I don't know about the past, but currently it seems there is only one authenticated api, and that's the one between geth and the beacon chain.

Relevant places in geth:
The api type:
https://github.com/ethereum/go-ethereum/blob/f51f6edb40f714107cfba90d201cba0a97939dd3/rpc/types.go#L31-L38

You can also see the Public field is deprecated.

And the only time Authenticated is set:
https://github.com/ethereum/go-ethereum/blob/f51f6edb40f714107cfba90d201cba0a97939dd3/eth/catalyst/api.go#L40-L51

Thanks for posting those links. This is coming into focus now.

Concepts:

API namespaces (net, eth, debug, personal, engine, etc)

API interfaces (--http, --ws, --authrpc) e.g. ipc/hostport

public vs private namespaces

authenticated vs unauthenticated interfaces

The namespaces have been classified as either public (e.g. net) vs private (personal, debug, etc.). That concept has been recently removed and the Public field in the exported types deprecated as you have pointed out. ethereum/go-ethereum#25053 ethereum/go-ethereum#25059

The authenticated API appears to have been added because of the engine API used for beacon chain communications, which according to the spec "MUST" be on an authenticated interface that is also separate from the other API interfaces. https://github.com/ethereum/execution-apis/blob/main/src/engine/authentication.md

dex/networks/eth/common.go

client/asset/eth/multirpc.go

JoeGruffins · 2022-12-15T06:42:59Z

Attempt to make the config change clearer. Add context to dial. Update go.mod and use the geth jwt creation helper https://github.com/decred/dcrdex/compare/1037f299910880d40ff5452414dfafb89c1e166d..69be7a4d36477d63a4135edff0839c692b3b0d23

buck54321 · 2022-12-17T11:28:24Z

One comment regarding the UI: I think the extra JWT field should be be optional because it will generally be unused. Maybe have a little button next to the url input to add a JWT. If someone has multiple RPCs and each of the JWT field is empty, it'll be slightly distracting. Not a big deal though.

I agree with this, but it's definitely going to be a challenge. The ConfigOption system is just not built for this. One thing I was looking at some kind of "subform" like buck54321/dcrdex@6ec23d6...buck54321:dcrdex:repeatable-subforms. Then we can use the ShowByDefault field. It would take some work to get it right in forms.ts though, and I think it's worth its own effort.

chappjc

Tested dex-test (testnet server) with:

IPC path in markets.json (legacy / no changes)
IPC path in a new "/opt/geth/dex-settings.conf" file
addr=ws://host:port (authrpc) path and jwt= set in that new file

All worked as expected, and dex-test is currently running with geth node RPC via ws authrpc interface. However, for my first shot at JWT, I set jwt=/opt/geth/jwt.hex since I had that already in a file because geth was running with --authrpc.jwtsecret /opt/geth/jwt.hex. This was the error:

2022-12-27 22:04:39.755 [INF] DEX: Starting asset backend "eth"...
failed to start asset "eth": connect failure: unable to create auth function: encoding/hex: invalid byte: U+002F '/'

I realized quickly that I needed to put the hex chars in the conf file instead, but the error was a little confusing because it wasn't prefixed with something like "invalid jwt hex bytes: %w". I figure it should have the ability to read the jwt from an existing jwt.hex file as well, but this is fine.

Also regarding the server, it hurts to suggest it, but for future work we should facilitate use of third-party http RPC providers. An operator may decide to pay for a premium account with something like Ankr, and if that's what it takes to get adoption for ETH by operators, so be it.

JoeGruffins · 2022-12-28T08:34:10Z

Just rebased.

Add ability for client and server to connect over authenticated websocket to a geth full node.

JoeGruffins · 2022-12-28T11:47:46Z

jwt in the config can be hex or a file pointing to the hex https://github.com/decred/dcrdex/compare/b33f24b7bfad6f0c670b41621e0bfd6b72882585..5e57a58a66591e875221e8ddcb829f51b4eeafe1

JoeGruffins force-pushed the simnetethok branch 6 times, most recently from 4290121 to 6849f41 Compare November 25, 2022 07:10

JoeGruffins commented Nov 25, 2022

View reviewed changes

JoeGruffins force-pushed the simnetethok branch from 6849f41 to f849ce0 Compare November 25, 2022 07:39

JoeGruffins marked this pull request as ready for review November 25, 2022 07:48

chappjc added the ETH label Nov 30, 2022

martonp reviewed Nov 30, 2022

View reviewed changes

client/webserver/site/src/js/forms.ts Outdated Show resolved Hide resolved

client/asset/eth/eth.go Outdated Show resolved Hide resolved

client/webserver/site/src/js/forms.ts Outdated Show resolved Hide resolved

JoeGruffins force-pushed the simnetethok branch 2 times, most recently from bf485b4 to 0c9788b Compare December 1, 2022 06:49

JoeGruffins force-pushed the simnetethok branch from 0c9788b to d2a1986 Compare December 1, 2022 07:31

chappjc reviewed Dec 13, 2022

View reviewed changes

JoeGruffins force-pushed the simnetethok branch 2 times, most recently from 1037f29 to 69be7a4 Compare December 15, 2022 06:39

JoeGruffins force-pushed the simnetethok branch from 69be7a4 to 6ec23d6 Compare December 15, 2022 07:26

chappjc self-requested a review December 19, 2022 22:28

chappjc added this to the 0.6 milestone Dec 27, 2022

chappjc reviewed Dec 27, 2022

View reviewed changes

JoeGruffins force-pushed the simnetethok branch from 6ec23d6 to b33f24b Compare December 28, 2022 08:34

eth: Add authenticated geth rpc capabilities.

5e57a58

Add ability for client and server to connect over authenticated websocket to a geth full node.

JoeGruffins force-pushed the simnetethok branch from b33f24b to 5e57a58 Compare December 28, 2022 11:47

chappjc merged commit 75e8aac into decred:master Dec 29, 2022

chappjc mentioned this pull request Jan 2, 2023

eth/server: Problem getting balances. #2016

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

eth: Add authenticated geth rpc capabilities. #1963

eth: Add authenticated geth rpc capabilities. #1963

JoeGruffins commented Nov 17, 2022 •

edited

Loading

JoeGruffins left a comment

JoeGruffins Nov 25, 2022

JoeGruffins Nov 25, 2022

JoeGruffins Nov 25, 2022

buck54321 Dec 17, 2022

JoeGruffins Nov 25, 2022

JoeGruffins Nov 25, 2022

chappjc Dec 13, 2022 •

edited

Loading

JoeGruffins Dec 14, 2022

martonp left a comment •

edited

Loading

JoeGruffins commented Dec 1, 2022

JoeGruffins commented Dec 1, 2022

chappjc Dec 13, 2022 •

edited

Loading

chappjc Dec 13, 2022

JoeGruffins Dec 14, 2022

chappjc Dec 14, 2022

JoeGruffins Dec 15, 2022

chappjc Dec 13, 2022 •

edited

Loading

JoeGruffins Dec 14, 2022

JoeGruffins Dec 14, 2022

chappjc Dec 14, 2022 •

edited

Loading

JoeGruffins commented Dec 15, 2022

buck54321 commented Dec 17, 2022 •

edited

Loading

chappjc left a comment •

edited

Loading

JoeGruffins commented Dec 28, 2022

JoeGruffins commented Dec 28, 2022


		cfg := new(config)

		// NOTE: ipc only is deprecated. Remove this at some point.

		export DELTA_HTTP_PORT="38556"
		export DELTA_WS_PORT="38557"

eth: Add authenticated geth rpc capabilities. #1963

eth: Add authenticated geth rpc capabilities. #1963

Conversation

JoeGruffins commented Nov 17, 2022 • edited Loading

JoeGruffins left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chappjc Dec 13, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martonp left a comment • edited Loading

Choose a reason for hiding this comment

JoeGruffins commented Dec 1, 2022

JoeGruffins commented Dec 1, 2022

chappjc Dec 13, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chappjc Dec 13, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chappjc Dec 14, 2022 • edited Loading

Choose a reason for hiding this comment

JoeGruffins commented Dec 15, 2022

buck54321 commented Dec 17, 2022 • edited Loading

chappjc left a comment • edited Loading

Choose a reason for hiding this comment

JoeGruffins commented Dec 28, 2022

JoeGruffins commented Dec 28, 2022

JoeGruffins commented Nov 17, 2022 •

edited

Loading

chappjc Dec 13, 2022 •

edited

Loading

martonp left a comment •

edited

Loading

chappjc Dec 13, 2022 •

edited

Loading

chappjc Dec 13, 2022 •

edited

Loading

chappjc Dec 14, 2022 •

edited

Loading

buck54321 commented Dec 17, 2022 •

edited

Loading

chappjc left a comment •

edited

Loading