pokt-network/pocket

[IBC] Track and Include the IBC store in the network state (ICS-24)

h5law opened this issue · 2 comments

h5law commented

Objective

Add a new state tree that tracks the IBC store state for the entire chain.

Implement IBC message types that fulfil the Message interface defined in utility. These messages should define the update of key-value pairs in the IBC stores or the removal of a key from the local IBC store.

These messages should be propagated through the network and upon their receipt each node should add them to the transactions mempool managed by utility in order for them to be included in a new block and ultimately change the IBC state tree. This should include the logic to add any changes to a postgres table and from this set of updates update the IBC store state tree to include these changes.

Origin Document

IBC Store Outline

Goals

  • Support the tracking and inclusion of IBC related transition events in the Pocket blockchain by leveraging the existing message handler design in the utility module
    • Add IBC store state tree to the TreeStore
    • Create an IBC message type and handler method in the IBC module to propogate the changes to the IBC store
    • Implement the utility Message interface for the IBC message types for their use in the Transactions Mempool
    • Integrate the IBC message handling into the block production/application logic
  • Implement a store manager to interact with the IBC store state tree locally to each host and send IBC messages on any changes

Deliverable

  • Add IBC store state tree
  • Add IBC message types
  • Add IBC message handler
  • Add IBC message -> utility message conversion
    • Integrate mempool reaping with block production/application
  • Add IBC store manager to interact with the stores locally and propagate IBC messages on changes

Non-goals / Non-deliverables

  • Change any of the existing block production/application logic

General issue deliverables

  • Update the appropriate CHANGELOG(s)
  • Update any relevant local/global README(s)
  • Update relevant source code tree explanations
  • Add or update any relevant or supporting mermaid diagrams

Testing Methodology

  • Task specific tests or benchmarks: make ...
  • New tests or benchmarks: make ...
  • All tests: make test_all
  • LocalNet: verify a LocalNet is still functioning correctly by following the instructions at docs/development/README.md
  • k8s LocalNet: verify a k8s LocalNet is still functioning correctly by following the instructions here

Creator: @h5law
Co-Owners: @Olshansk

Tasks

No tasks being tracked yet.

@h5law Regarding the goals & deliverables

Create a seperate mempool for the IBC store transition events

I think the goal is to keep track of IBC state transition events that haven't been committed. How that's done (same mempool, other mempool, magic) shouldn't be a specific goal/deliverable unless we've decided on a path forward.

Create an IBC message type and handler method in the IBC module to propogate the changes to the IBC store
Implement the reaping of the IBC mempool in the block production/application logic

Just a NIT (no need to change it here but for future reference). These are very good an actionable deliverables. But I would say that the goal is "Support the tracking and inclusion of IBC related transition events in the Pocket blockchain by leveraging the existing message handler design in the utility module`

If you've ever unsure of what the exact plan is just call it "Consider implementing X via Y or Z but a different approach". Happy to re-review the ticket if after any changes, just lmk


Inserting a screenshot of what we had prior for future reference

Screenshot 2023-06-20 at 10 24 35 AM

h5law commented

I think the goal is to keep track of IBC state transition events that haven't been committed. How that's done (same mempool, other mempool, magic) shouldn't be a specific goal/deliverable unless we've decided on a path forward.

@Olshansk I have updated the issue and the title. I have decided to use the existing TxMempool and moving forward I think this will be the best approach. I will document the reasons why in the ibc/docs folder when the PR is ready to merge.