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

Revert EventAttribute types #85

Closed
wants to merge 4 commits into from
Closed

Revert EventAttribute types #85

wants to merge 4 commits into from

Conversation

LCyson
Copy link
Contributor

@LCyson LCyson commented Mar 15, 2023

Describe your changes and provide context

This PR reverts the following RPC data type changes that affect both FE and IBC channel queries:

The intention to revert them is because we found that these changes conflict with most of existing cosmos auxiliary packages (including FE / IBC hermes / tendermint-rs and potentially more).

Testing performed to validate your change

  • unit tests
  • Pending e2e test on local cluster and devnet-3

Copy link
Contributor

@BrandonWeng BrandonWeng left a comment

Choose a reason for hiding this comment

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

Change seems fine but similar comment here: sei-protocol/sei-cosmos#195 (review)

Could we test this out in a loadtest cluster before deploying to sei-devnet-3?

Copy link
Contributor

@philipsu522 philipsu522 left a comment

Choose a reason for hiding this comment

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

LGTM. we probably want to not only test this but also do an upgrade to this version from 2.0.40beta

@LCyson LCyson closed this Mar 29, 2023
Timwood0x10 pushed a commit to Timwood0x10/sei-tendermint that referenced this pull request Jun 7, 2023
…tocol#85)

## Describe your changes and provide context
This adds some resources that were previously missing from the resource
tree. It also adds a unit test that requires ALL defined resource proto
enum values to be keys in the resource tree.

## Testing performed to validate your change
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.

3 participants