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

Add basic event stream RPC interface docs and JSON/RPC client lib #523

Merged
merged 34 commits into from
Jan 24, 2025

Conversation

peterbroadhurst
Copy link
Contributor

In PR chain with #515

  • Adds documentation
  • Consolidates rpcbackend JSON/RPC client from firefly-common into rpcclient to allow us to enhance it
    • Also has the benefit of eliminating lots of type mapping complexity
  • Enhances Subscribe() functionality of rpcclient.WSClient to support:
    • ptx_subscribe("receipts", "listenerName")
    • ptx_ack
    • ptx_nack

Signed-off-by: Peter Broadhurst <[email protected]>
Signed-off-by: Peter Broadhurst <[email protected]>
Signed-off-by: Peter Broadhurst <[email protected]>
…make eth_subscribe a sample

Signed-off-by: Peter Broadhurst <[email protected]>
Signed-off-by: Peter Broadhurst <[email protected]>
Signed-off-by: Peter Broadhurst <[email protected]>
…rder of numbers with commits

Signed-off-by: Peter Broadhurst <[email protected]>
Signed-off-by: Peter Broadhurst <[email protected]>
@peterbroadhurst peterbroadhurst requested a review from hosie January 22, 2025 04:34
@peterbroadhurst peterbroadhurst changed the title Es rpc docs Add basic event stream RPC interface docs and JSON/RPC client lib Jan 22, 2025
@peterbroadhurst peterbroadhurst marked this pull request as draft January 22, 2025 19:16
@peterbroadhurst
Copy link
Contributor Author

Putting back into draft to add to E2E test for full validation

@peterbroadhurst
Copy link
Contributor Author

Added E2E test, and worked through:

  • Mismatch in nesting of result in the payloads by convention with Ethereum notifications
  • Not attempting to post-filter on sequence, as the pre-insertion trigger does not have access to the sequence

@peterbroadhurst peterbroadhurst marked this pull request as ready for review January 23, 2025 04:55
Signed-off-by: Peter Broadhurst <[email protected]>
…iption for notifications

Signed-off-by: Peter Broadhurst <[email protected]>
@peterbroadhurst
Copy link
Contributor Author

Attempted to drive one step closer to consistency with eth_subscribe:

// Note we attempt strong consistency with etH_subscribe semantics here, as described in https://geth.ethereum.org/docs/interacting-with-geth/rpc/pubsub
// However, we have layered acks on top - so we're not 100%.
// We also end up with quite a bit of nesting doing this:
// { "jsonrpc": "2.0", "method": "ptx_subscription",
// "params": {
// "subscription": "0xcd0c3e8af590364c09d0fa6a1210faf5",
// "result": {
// "batchId": 12345,
// "receipts": [ ... interesting stuff ]
// }
// }
// }
sub.ctrl.Send("ptx_subscription", &pldapi.JSONRPCSubscriptionNotification[pldapi.TransactionReceiptBatch]{

Signed-off-by: Peter Broadhurst <[email protected]>
Copy link
Contributor

@hosie hosie left a comment

Choose a reason for hiding this comment

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

LGTM. I left a few comments on #515 but nothing worth holding this out.

@hosie hosie enabled auto-merge January 24, 2025 10:53
@hosie hosie merged commit 66d8ca6 into main Jan 24, 2025
7 checks passed
@hosie hosie deleted the es-rpc-docs branch January 24, 2025 11:03
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