Test for OpenRPC subscription methods on trace/debug modules #356

ziogaschr · 2021-04-12T11:32:01Z

meowsbits · 2021-04-12T12:10:13Z

Thanks for getting this going!
Have you tried generating docs with this and see if they are as expected? I haven't yet, but have a feeling there's going to be a mismatch; please correct me if I'm wrong.

Pub/Sub is not handled well automatically by the reflector; it's a "special" case for the way ethereum/go-ethereum does (or does not) do method reflection and mapping.

For example, eth_subscribe("newHeads") uses the method SubscribeNewHeads (or is it NewHeads? can't remember exactly); does trace_filter work the same (or use (api *TraceAPI) Filter(...))?

The current hack-around for eth_subscribe's weirdness is the RPCSubscription mocks in node/openrpc.go, where the subscription's basically-enum values (eg. newHeads, newSideHeads, newPendingTransactions...) are provided in hardcode as type-mappings (see rpcSubscriptionParamsNameD).

I think we may need to either provide a similar kind of mock pattern for debug_ and trace_ modules (as you have started), or get a little fancier with additional conditions to the catch-all pub/sub skip logic.

TL;DR: I don't think debug_subscribe and trace_subscribe exist.

	        // Exclude methods that handle subscriptions, but do so without adhering to the conventional code pattern.
		// Eg. *filters.PublicFiltersAPI.SubscribeNewHeads handles eth_subscribe("newHeads"), but there
		// isn't a method called `eth_subscribeNewHeads`. So we blacklist all these methods and use
		// the mock subscription receiver type RPCSubscription.
		// This pattern matches all strings that start with Subscribe and are suffixed with a non-zero
		// number of A-z characters.
		if isPubSub(method.Type) {
			// This catches MOST of them (except SubscribeSyncStatus)
			return false
		}

ziogaschr · 2021-04-12T12:16:34Z

No I haven't generated the docs. I will.
I think I see what you mean now. Will have a look myself and I will ping you.

Thanks man

ziogaschr · 2021-04-21T14:41:40Z

node/openrpc.go

@@ -329,7 +369,8 @@ var blockNumberOrHashD = fmt.Sprintf(`{
          ]
        }`, blockNumberD, commonHashD, requireCanonicalD)

-var rpcSubscriptionParamsNameD = `{
+var rpcEthSubscriptionParamsNameD = `{
+		"title": "subscriptionName",


@meowsbits added this, as it wasn't working for params without it even for eth_subscribe.

ziogaschr · 2021-04-21T14:44:52Z

node/openrpc.go

@@ -208,25 +210,59 @@ func newOpenRPCDocument() *go_openrpc_reflect.Document {
 }

 /*
-The following struct RPCSubscription and RPCSubscription.Unsubscribe method
+The following structs *RPCSubscription and *RPCSubscription.Unsubscribe method


@meowsbits I want to play a bit in order to find a better way of merging those 3 duplicated implementations where only the subscriptionName type in subscribe changes.

Let me know if you have an idea.

Have you experimented at all with getting rid of or modifying the general "skip PubSub" case to be more permissive (even if only allowing debug and trace receivers through)?

No, I haven't. I will give it a try. Thanks

I have tried it here, which adds the trace_filter nicely, although it ~~can~~can't be used in the way documented, which leads to issues. What do you think? Did you mean something else @meowsbits?

X-linking: #364 (review)

ziogaschr · 2021-04-21T14:45:30Z

node/openrpc.go

+var rpcDebugSubscriptionParamsNameD = `{
+		"title": "subscriptionName",
+		"oneOf": [
+			{"type": "string", "enum": ["traceFilter"], "description": "Returns traces for the filtered transactions within a range of blocks."}


I still have to collect all the debug_* subscription based methods

ziogaschr · 2021-04-21T14:57:20Z

Generated docs sample

@meowsbits

I want to allow debug and trace API modules through. Taken from @meowsbits d7194b5

…ble methods

ziogaschr · 2021-04-27T16:11:03Z

@meowsbits I added generating docs for both *_subscribe, but also more accurate logs for the subscriptionable methods like trace_filter (in this case, I change the examples section).

I still need to fix the examples...
I am thinking if it makes sense in this case to add a new field per method Subscription: bool

meowsbits · 2021-04-27T16:19:27Z

Per this comment at OpenRPC/spec we could potentially use x-subscription: true (or similar) as a specification extension on the method annotations.

…nable methods

ziogaschr · 2021-04-29T22:22:06Z

@meowsbits can you review it? To see what's done, check the trace_subscribe and trace_filter in trace.md file.

p.s.: At the moment I haven't enabled the same for the eth package, but we can do that there too. For example documenting the eth_newHeads.

Per this comment at OpenRPC/spec we could potentially use x-subscription: true (or similar) as a specification extension on the method annotations.

This is a great addon, but not straightforward to implement. As discussed, let's keep this for later.

Adapts the canonical list of methods that should be provided by the document to meet what the document says. Date: 2021-04-30 06:13:54-05:00 Signed-off-by: meows <b5c6@protonmail.com>

meowsbits

If this latest commit (ce80270) looks OK to you, everything LGTM!

(I've left the methods commented because that lists serves a duty both as a test case, but also as a "behind-the-scenes" developer reference.)

…well as better examples

…*filters.PublicFilterAPI` methods

ziogaschr added 2 commits April 8, 2021 01:36

ethclient: remove duplicate rpc method name in tests

cc17ccf

ethclient, node: test for subscription methods in openrpc discovery

b36338d

ziogaschr requested a review from meowsbits April 12, 2021 11:32

ziogaschr changed the title ~~Merge/foundation release/1.10.1 resolved state diff openrpc~~ Test for OpenRPC subscription methods on trace/debug modules Apr 12, 2021

ziogaschr added A3-inprogress ⏳ F4-tests 💻 labels Apr 12, 2021

Base automatically changed from merge/foundation-release/1.10.1-resolved-stateDiff to merge/foundation-release/1.10.1-resolved April 13, 2021 13:23

ziogaschr added 4 commits April 21, 2021 16:45

ethclient: remove whitespace at EOL

a8723f7

node: add openrpc schema mapper for subscription IDs

edecf8e

node: add missing title for rpcSubscriptionParamsNameD schema mapper

3ac09e5

node: add openrpc schema mappers for debug, trace subscription methods

e4f852a

ziogaschr commented Apr 21, 2021

View reviewed changes

node: update openrpc schema mapper methods

7c81d24

meowsbits mentioned this pull request Apr 23, 2021

WIP: Test for OpenRPC subscription methods on trace/debug modules / whitelist-style pubsub skips #364

Closed

ziogaschr added 3 commits April 27, 2021 19:04

node: whitelist skip pubsub to eth_ public api

733c7dc

I want to allow debug and trace API modules through. Taken from @meowsbits d7194b5

ethclient: generate docs for both *_subscribe as well Subscriptiona…

47f2883

…ble methods

docs/JSON-RPC-API/modules: run 'make docs-generate'

9a45a7a

ziogaschr added 2 commits April 30, 2021 01:11

ethclient: remove js console example from generated docs for subscrio…

6c986ff

…nable methods

docs/JSON-RPC-API/modules: run 'make docs-generate'

a9d4be9

ethclient: fixup test again canonical methods list

ce80270

Adapts the canonical list of methods that should be provided by the document to meet what the document says. Date: 2021-04-30 06:13:54-05:00 Signed-off-by: meows <b5c6@protonmail.com>

meowsbits approved these changes Apr 30, 2021

View reviewed changes

node: openrpc to return the *filters.PublicFilterAPI methods

ec1e00c

ziogaschr added 3 commits April 30, 2021 19:25

ethclient: generated docs have *filters.PublicFilterAPI methods as …

c7b1b15

…well as better examples

docs/JSON-RPC-API/modules: run 'make docs-generate'

82b91f0

ethclient: bring up tests against canonical methods list, including `…

49d5ab8

…*filters.PublicFilterAPI` methods

ziogaschr merged commit 695c0a2 into merge/foundation-release/1.10.1-resolved May 4, 2021

ziogaschr deleted the merge/foundation-release/1.10.1-resolved-stateDiff-openrpc branch May 4, 2021 10:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Test for OpenRPC subscription methods on trace/debug modules #356

Test for OpenRPC subscription methods on trace/debug modules #356

ziogaschr commented Apr 12, 2021

meowsbits commented Apr 12, 2021 •

edited

Loading

ziogaschr commented Apr 12, 2021

ziogaschr Apr 21, 2021

ziogaschr Apr 21, 2021

meowsbits Apr 21, 2021

ziogaschr Apr 22, 2021

ziogaschr Apr 22, 2021 •

edited

Loading

meowsbits Apr 23, 2021

ziogaschr Apr 21, 2021

ziogaschr commented Apr 21, 2021

ziogaschr commented Apr 27, 2021

meowsbits commented Apr 27, 2021 •

edited

Loading

ziogaschr commented Apr 29, 2021

meowsbits left a comment •

edited

Loading

Test for OpenRPC subscription methods on trace/debug modules #356

Test for OpenRPC subscription methods on trace/debug modules #356

Conversation

ziogaschr commented Apr 12, 2021

meowsbits commented Apr 12, 2021 • edited Loading

ziogaschr commented Apr 12, 2021

ziogaschr Apr 21, 2021

Choose a reason for hiding this comment

ziogaschr Apr 21, 2021

Choose a reason for hiding this comment

meowsbits Apr 21, 2021

Choose a reason for hiding this comment

ziogaschr Apr 22, 2021

Choose a reason for hiding this comment

ziogaschr Apr 22, 2021 • edited Loading

Choose a reason for hiding this comment

meowsbits Apr 23, 2021

Choose a reason for hiding this comment

ziogaschr Apr 21, 2021

Choose a reason for hiding this comment

ziogaschr commented Apr 21, 2021

Generated docs sample

ziogaschr commented Apr 27, 2021

meowsbits commented Apr 27, 2021 • edited Loading

ziogaschr commented Apr 29, 2021

meowsbits left a comment • edited Loading

Choose a reason for hiding this comment

meowsbits commented Apr 12, 2021 •

edited

Loading

ziogaschr Apr 22, 2021 •

edited

Loading

meowsbits commented Apr 27, 2021 •

edited

Loading

meowsbits left a comment •

edited

Loading