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

Agoric Contract IBC Middleware #8583

Closed
Tracked by #8903 ...
sufyaankhan opened this issue Nov 30, 2023 · 8 comments · Fixed by #8624
Closed
Tracked by #8903 ...

Agoric Contract IBC Middleware #8583

sufyaankhan opened this issue Nov 30, 2023 · 8 comments · Fixed by #8624
Assignees
Labels

Comments

@sufyaankhan
Copy link

sufyaankhan commented Nov 30, 2023

What is the Problem Being Solved?

Ordinary ICS-20 IBC transfers use connections on the transfer port; these connections are handled by an ibc module provided by the interchain stack. It's not straightforward (or possible?) for the swingset network API to take over such connections. So in order for contracts to receive such messages, we need some other solution.

Description of the Design

Expose IBC go's Transfer App inbound packets to JS Smart Contracts, as a form of IBC middleware.

There are 3 pieces of work (boxes in RED in the diagram) that need to be implemented.

1. Agoric Contract Middleware

  • Agoric Contract Middleware is a wrapper for IBC Contract app Channels. It intercepts inbound and outbound transfer packets.

  • If a packet contains conventional memo fields that indicates an Agoric contract target, then forward the packet details and target information to x/vtransfer.

2. x/vtransfer

  • This module simply forwards messages to swingset over the bridge device.

3. Transfer Notification Service

  • Maintains a table of target names mapped to publishers.
  • When receiving message from x/vtransfer, lookup the target in the table and publish the packet to the target.
Screenshot 2023-11-30 at 8 33 19 AM

Security Considerations

  • For Transfer Notification Service - Registering or removing targets are privileged operations. (accessible by core-eval)
  • Contract middleware must be robust so transfer is not impeded

Scaling Considerations

  • Transfer Notification Services target collections should be durable, so that they will scale over time.

Test Plan

  • Unit Test + Manual E2E testing (automated)

Upgrade Considerations

  • Upgradable Transfer Notification Services
@dckc
Copy link
Member

dckc commented Dec 22, 2023

Problem

We need to expose IBC go's Transfer App inbound packets to JS Smart Contracts.

@sufyaankhan @michaelfig could you elaborate on that? Why do we need to do that?

cc @schnetzlerjoe

@dckc
Copy link
Member

dckc commented Dec 22, 2023

based on discussion with @sufyaankhan and @michaelfig , I gather the problem we're addressing is this:

Ordinary ICS-20 IBC transfers use connections on the transfer port; these connections are handled by an ibc module provided by the interchain stack. It's not straightforward (or possible?) for the swingset network API to take over such connections. So in order for contracts to receive such messages, we need some other solution.

@schnetzlerjoe
Copy link
Contributor

based on discussion with @sufyaankhan and @michaelfig , I gather the problem we're addressing is this:

Ordinary ICS-20 IBC transfers use connections on the transfer port; these connections are handled by an ibc module provided by the interchain stack. It's not straightforward (or possible?) for the swingset network API to take over such connections. So in order for contracts to receive such messages, we need some other solution.

This would be correct! We (Calypso) COULD get away for now probably without but the concern with engineering within is that over time the uncomposability with the transfer port especially with PFM would draw away teams like Skip or create stray asset issuances. I.e: I use skip API to route asset, ends up on another port not transfer. Now uncomposable with Calypso or any other protocol on Agoric.

@dckc
Copy link
Member

dckc commented Jan 11, 2024

does anybody have a user story sketch of how this works?
maybe even a sequence diagram?

@michaelfig
Copy link
Member

maybe even a sequence diagram?

@raphdev and I are discussing this today. There will be artifacts.

@dckc dckc changed the title Agoric Contract Middleware Agoric Contract IBC Middleware Jan 11, 2024
@raphdev
Copy link
Contributor

raphdev commented Feb 14, 2024

I don't have detailed sequence diagrams, mainly modified versions of @sufyaankhan's diagrams as an aid for documenting architecture:

What exists today (as far as code, not on-chain):

image

Addition of transfer middleware:

image

And a view outlining kernel/cosmos boundary:

image

@dckc
Copy link
Member

dckc commented Apr 23, 2024

TIL: IBC middleware pre-dates this issue.

@raphdev
Copy link
Contributor

raphdev commented Apr 23, 2024

I believe what we call "Agoric Contract Middleware" is the custom IBC middleware. Originally, the custom middleware and x/transfer were separate components, but they ended up being merged.

@mergify mergify bot closed this as completed in #8624 Jun 8, 2024
mergify bot added a commit that referenced this issue Jun 8, 2024
closes: #8583, closes: #9059, closes: #9256

## Description

This PR adds the vTransfer middleware module which acts as a
notification module on the transfer port for contracts that need to act
based on it.

### Security Considerations

As discussed, this does change how inbound assets are handled. 

### Testing Considerations

The middleware module has mock unit tests in ibc_middleware_test.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants