-
Notifications
You must be signed in to change notification settings - Fork 12
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
[Off-Chain] Add EventsReplayClient
Generic Event Listener for BlockClient
and DelegationClient
#220
Conversation
2f914dc
to
c528d8c
Compare
EventDelegateeChange
event emission and cache invalidationMappedClient
Generic Event Listener for BlockClient
and DelegationClient
@h5law Skipping this over for now but please request a review when we should take look. |
MappedClient
Generic Event Listener for BlockClient
and DelegationClient
EventsReplayClient
Generic Event Listener for BlockClient
and DelegationClient
66f379f
to
2ea2cae
Compare
5fad821
to
5e2f32d
Compare
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 finally got through it! 😅🎉
This PR is in great shape, especially for its size. I have one more round of comments and then I think it'll be good to go.
I also wanted to call out #220 (comment) and #220 (comment) to make sure that we don't loose track of them.
@Olshansk @bryanchriswhite Should be good to go now <3 |
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.
Preemptively approving but I have one last change request regarding the sequence diagram.
Thanks for all your patience! 🙈
All comments addressed, discussed offline
…#253) This is aiming to fix multiple issues in E2E tests and just general QoL improvements: 1. Adding a comment in `relay.feature` about the need to run `make supplier1_stake && make app1_stake` until we fix #180 (known issue) 2. Replacing the reference to host `sequencer-poktroll-sequencer` with `localhost` (see first screenshot below) 3. Fixed a bug leading us to assert against a `MsgSubmitProof` event instead of a `MsgCreateClaim` event (see second screenshot below) 4. Fixed on-chain logging under `x/*` by replacing `logger\.(Info|Error)\("([^"]+)"(?:, (.*))?\)` with `logger.$1(fmt.Sprintf("$2", $3))` so it works as expected 5. Some quality-of-life comments & TODOs 6. Minor additions to unit tests from #220
commit df73cfa Author: Bryan White <[email protected]> Date: Thu Dec 14 01:06:03 2023 +0100 refactor: `NewMinedRelay` to shared testutil (#262) commit 410965a Author: Bryan White <[email protected]> Date: Thu Dec 14 01:05:52 2023 +0100 fix: PR template typo 2 (#269) commit dd7df68 Author: Daniel Olshansky <[email protected]> Date: Wed Dec 13 13:45:26 2023 -0800 [Testing] Unit tests, E2E tests, logging and other Fixes / "Touchups" (#253) This is aiming to fix multiple issues in E2E tests and just general QoL improvements: 1. Adding a comment in `relay.feature` about the need to run `make supplier1_stake && make app1_stake` until we fix #180 (known issue) 2. Replacing the reference to host `sequencer-poktroll-sequencer` with `localhost` (see first screenshot below) 3. Fixed a bug leading us to assert against a `MsgSubmitProof` event instead of a `MsgCreateClaim` event (see second screenshot below) 4. Fixed on-chain logging under `x/*` by replacing `logger\.(Info|Error)\("([^"]+)"(?:, (.*))?\)` with `logger.$1(fmt.Sprintf("$2", $3))` so it works as expected 5. Some quality-of-life comments & TODOs 6. Minor additions to unit tests from #220 commit d621631 Author: Redouane Lakrache <[email protected]> Date: Wed Dec 13 16:24:34 2023 +0100 [SDK] feat: Have distinct JSON-RPC and gRPC urls (#261) * feat: Have distinct JSON-RPC and gRPC urls * chore: Trigger e2e tests * chore: Fix import groups commit 4e30e27 Author: Bryan White <[email protected]> Date: Wed Dec 13 14:28:36 2023 +0100 fix PR template testing checklist item (#268)
…#253) This is aiming to fix multiple issues in E2E tests and just general QoL improvements: 1. Adding a comment in `relay.feature` about the need to run `make supplier1_stake && make app1_stake` until we fix #180 (known issue) 2. Replacing the reference to host `sequencer-poktroll-sequencer` with `localhost` (see first screenshot below) 3. Fixed a bug leading us to assert against a `MsgSubmitProof` event instead of a `MsgCreateClaim` event (see second screenshot below) 4. Fixed on-chain logging under `x/*` by replacing `logger\.(Info|Error)\("([^"]+)"(?:, (.*))?\)` with `logger.$1(fmt.Sprintf("$2", $3))` so it works as expected 5. Some quality-of-life comments & TODOs 6. Minor additions to unit tests from #220
…#253) This is aiming to fix multiple issues in E2E tests and just general QoL improvements: 1. Adding a comment in `relay.feature` about the need to run `make supplier1_stake && make app1_stake` until we fix #180 (known issue) 2. Replacing the reference to host `sequencer-poktroll-sequencer` with `localhost` (see first screenshot below) 3. Fixed a bug leading us to assert against a `MsgSubmitProof` event instead of a `MsgCreateClaim` event (see second screenshot below) 4. Fixed on-chain logging under `x/*` by replacing `logger\.(Info|Error)\("([^"]+)"(?:, (.*))?\)` with `logger.$1(fmt.Sprintf("$2", $3))` so it works as expected 5. Some quality-of-life comments & TODOs 6. Minor additions to unit tests from #220
Summary
Human Summary
This PR converts the existing
BlockClient
into an implenentation of the genericEventsReplayClient
which subscribes to and listens for on chain events and maps them into the desired type of the implementedEventsReplayClient
BlockClient ->
EventsReplayClient[Block]
DelegationClient ->
EventsReplayClient[DelegateeChange]
These new client interfaces have their own methods which wrap those exposed by the
EventsReplayClient
AI Summary
Summary generated by Reviewpad on 07 Dec 23 20:34 UTC
This pull request:
ring_signer.go
file in thepkg/signer
package by updating the comments for theNewRingSigner
function and theSign
method.client_integration_test.go
file in thepkg/client/delegation
package, which contains a test function for theDelegationClient
redelegation functionality. However, the test is currently skipped and needs further investigation and fixing.replay_client.go
in thepkg/client/events
package, which implements a replay client for handling events and provides various functions for interacting with the client.claim.go
file under thepkg/relayer/session
package, involving refactoring and optimization of code.errors.go
file, which contained a package namedwebsocket
and imported another package namederrorsmod
, as well as defined an error variable and a codespace variable.godoc.go
in thepkg/client/block
package, which is a wrapper for theEventsReplayClient[Block]
generic and listens for committed block events on the chain.event.proto
in thepocket/application
directory, defining a protocol buffer messageEventRedelegation
for application delegatee gateway changes on the chain.errors.go
with error handling functionality specific to event handling in the client package.client_test.go
with test cases for theDelegationClient
struct.websocket
that provides a websocket client for connecting to a cosmos-sdk based chain node.godoc.go
that contains documentation for thedelegation
package and its functionalities.redelegation.go
that defines a struct and a factory function for deserializing data into the struct.client_integration_test.go
file, including changes in the package name and import statements.events_query.md
file that provided an overview of the package.Please let me know if you need further assistance or have any questions.
Issue
Type of change
Select one or more:
Testing
make go_develop_and_test
Sanity Checklist