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

[IBC] Change Events to not have a Height field and use uint64 in queries #931

Merged
merged 9 commits into from
Jul 26, 2023

Conversation

h5law
Copy link
Contributor

@h5law h5law commented Jul 24, 2023

Description

Summary generated by Reviewpad on 26 Jul 23 09:16 UTC

This pull request includes multiple file diffs with various changes:

  1. The newPersistenceMock function in the file provable_store_test.go has been modified. The second input parameter has been changed from int64 to uint64, affecting the implementation of the function and the GetIBCStoreEntry method call.

  2. A new file called ibc_events.go has been added in the shared/core/types directory. The file contains a new package called types and a new function called NewAttribute which takes two byte array arguments and returns a pointer to an Attribute struct.

  3. In the import path coreTypes of the core_types package in the github.com/pokt-network/pocket/shared/core/types package, the import path has been changed to core_types. Additionally, the method signatures for the EmitEvent and QueryEvents functions have been updated to use the updated core_types.IBCEvent type. A new variable currHeight has been added to retrieve the current height.

  4. The file ibc_event_module.go in the shared/modules directory has been modified. The import path coreTypes has been changed to core_types. The EmitEvent and QueryEvents functions now accept and return the IBCEvent type from the core_types package instead of the coreTypes package.

  5. The file persistence/types/ibc.go has been modified, changing the type of the height parameter in the InsertIBCStoreEntryQuery, InsertIBCEventQuery, and GetIBCStoreEntryQuery functions from int64 to uint64.

  6. Changes have been made to the ibc_test.go file, including the modification of the data type of the height variable in the TestIBC_GetIBCStoreEntry test case from int64 to uint64. Test function names have been renamed, assignments of the height variable have been removed, and the logic to set the height value in the TestGetIBCEvent function has been modified.

  7. A new function GetRevisionNumber() has been added to a file, returning a uint64. A placeholder comment for the GetSupportedChains function has also been added.

  8. The file ibc_events.proto has been modified, changing the IBCEvent message and the Attribute message. The IBCEvent message now has a topic field of type string and a repeated attributes field of type Attribute.

  9. Changes have been made to the Get function in the provable_store.go file. The type of the currHeight variable has been changed from int64 to ConsensusHeight, requiring a conversion to int64 using the int64() function before passing it to NewReadContext.

  10. The persistence/ibc.go file has been modified in the persistence package. The SetIBCStoreEntry and SetIBCEvent functions have p.Height parameters casted to uint64 in the tx.Exec statement. The GetIBCStoreEntry function has the height parameter changed from int64 to uint64.

  11. Changes have been made to the PersistenceReadContext interface in the persistence_module.go file. A new GetRevisionNumber method has been added, taking an int64 height as input and returning a uint64 representing the revision number. The GetIBCStoreEntry method has changed the height parameter type from int64 to uint64. A new GetIBCEvents method has been added, taking a uint64 height and a string topic as input and returning a slice of IBCEvent objects.

Please review and implement the necessary changes for these updates.

Issue

Fixes N/A

Type of change

Please mark the relevant option(s):

  • New feature, functionality or library
  • Bug fix
  • Code health or cleanup
  • Major breaking change
  • Documentation
  • Other

List of changes

  • Remove the Height field from the IBCEvent proto
  • Pass in uint64 types instead of int64 to queries

Testing

  • make develop_test; if any code changes were made
  • make test_e2e on k8s LocalNet; if any code changes were made
  • e2e-devnet-test passes tests on DevNet; if any code was changed
  • Docker Compose LocalNet; if any major functionality was changed or introduced
  • k8s LocalNet; if any infrastructure or configuration changes were made

Required Checklist

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added, or updated, godoc format comments on touched members (see: tip.golang.org/doc/comment)
  • I have tested my changes using the available tooling
  • I have updated the corresponding CHANGELOG

If Applicable Checklist

  • I have updated the corresponding README(s); local and/or global
  • I have added tests that prove my fix is effective or that my feature works
  • I have added, or updated, mermaid.js diagrams in the corresponding README(s)
  • I have added, or updated, documentation and mermaid.js diagrams in shared/docs/* if I updated shared/*README(s)

@h5law h5law added code health Nice to have code improvement e2e-devnet-test Runs E2E tests on devnet ibc IBC specific changes labels Jul 24, 2023
@h5law h5law added this to the M7: Pocket NoS (North Star) milestone Jul 24, 2023
@h5law h5law requested review from Olshansk and dylanlott July 24, 2023 10:11
@h5law h5law self-assigned this Jul 24, 2023
@reviewpad reviewpad bot added small Pull request is small waiting-for-review labels Jul 24, 2023
@@ -17,6 +17,11 @@ func (p *PostgresContext) GetVersionAtHeight(height int64) (string, error) {
return "", nil
}

// TODO(#882): Implement this function
Copy link
Member

Choose a reason for hiding this comment

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

cc @0xBigBoss for visibility & context

persistence/test/ibc_test.go Outdated Show resolved Hide resolved
@h5law h5law changed the base branch from ibc/proto-exploration-ics23 to main July 26, 2023 09:04
@reviewpad reviewpad bot added large Pull request is large and removed small Pull request is small labels Jul 26, 2023
@reviewpad reviewpad bot added small Pull request is small and removed large Pull request is large labels Jul 26, 2023
@h5law h5law merged commit fb86e26 into main Jul 26, 2023
12 checks passed
@h5law h5law deleted the ibc/event_height branch July 26, 2023 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code health Nice to have code improvement e2e-devnet-test Runs E2E tests on devnet ibc IBC specific changes small Pull request is small waiting-for-review
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants