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

orchestration transfer() vow should not resolve until the remote chain confirms #9783

Closed
turadg opened this issue Jul 25, 2024 · 2 comments · Fixed by #9820
Closed

orchestration transfer() vow should not resolve until the remote chain confirms #9783

turadg opened this issue Jul 25, 2024 · 2 comments · Fixed by #9820
Assignees
Labels
enhancement New feature or request

Comments

@turadg
Copy link
Member

turadg commented Jul 25, 2024

What is the Problem Being Solved?

transfer method in local-orchestration-account.js returns a vow that returns once the message is put in the outbox. The transfer isn't really complete until the remote chain confirms. Moreover the transfer may fail and in that case the vow should reject.

Same goes for cosmos-orchestration-account.js but that's "Not yet implemented". I wrote the PR title generically as a product requirement.

Description of the Design

Prior art: https://gist.github.com/michaelfig/280acc7d0bb1eb10bf7570929305bb80

Monitor transfers accepts a single “app” / “tap” at a time. If we use it in a platform-level service that watches outgoing transfers and resolves promises, we still need to support developers registering their own handlers. We could use the one slot with a generic handler that calls out to other handlers.

Security Considerations

Scaling Considerations

Test Plan

Upgrade Considerations

@dckc
Copy link
Member

dckc commented Aug 5, 2024

transfer() uses executeTx; docs for executeTx say it waits for an ack. Is that just not right?

/**
* Execute a batch of messages in order as a single atomic transaction
* and return the responses. If any of the messages fails, the entire
* batch will be rolled back. Use `typedJson()` on the arguments to get
* typed return values.
*
* @template {TypedJson[]} MT messages tuple (use const with multiple
* elements or it will be a mixed array)
* @param {MT} messages
* @returns {PromiseVowOfTupleMappedToGenerics<{
* [K in keyof MT]: JsonSafe<ResponseTo<MT[K]>>;
* }>}
*/
async executeTx(messages) {

@michaelfig
Copy link
Member

transfer() uses executeTx; docs for executeTx say it waits for an ack. Is that just not right?

A "response" (ResponseTo<MsgTransfer>) above is promptly returned indicating the local transaction has completed in the same block that the cosmos layer received it (without any cross-chain interaction). For MsgTransfer, that return consists of just a sequence number to track the transfer which has been submitted to the IBC layer, and is waiting to be relayed to another chain. An IBC "ack" is multiblock, from a relayer sending an inbound IBC acknowledgement or timeout packet back in a transaction to the initiating chain, which the VM can correlate via the matching sequence number as the reply to the outbound transfer packet.


A brief segue: here's the Monsterpiece Theatre I acted out for @turadg and @0xpatrickdev when explaining these separate layers:

Alice writes a note to Bob, stamps it with a sequence number, and puts the note in her outbox

Without #9820: Alice jumps the gun and says: "Message sent! Full speed ahead!" Curtains.

With #9820: Ray collects the notes from Alice's outbox, delivers one to Bob's inbox.

Bob reads Alice's note, and performs the requested work, however long it may take.

Bob puts a reply note to Alice in his outbox, stamped with Alice's sequence number.

Ray collects the notes from Bob's outbox, delivers one to Alice's inbox.

Alice reads Bob's note, and decides between:

Alice says: "Message confirmed! Full speed ahead!"
-or-
Alice says: "Abort! Abort!"

Curtains.

@mergify mergify bot closed this as completed in #9820 Aug 5, 2024
mergify bot added a commit that referenced this issue Aug 5, 2024
closes: #9783

## Description

Introduce a per-LOA `packetTools` to grant powers used by IBC-specific `makeIBCReplyKit` and `makeIBCTransferSender` which enables a `localOrchestrationAccount` to send a packet and wait for a corresponding reply packet (such as IBC acknowledgement or IBC timeout) in the context of a single localChainAccount address.

The most important files are `agoric-sdk/packages/orchestration/src/exos/packet-tools.js` and `ibc-packet.js` in the same directory.

### Security Considerations

This change is the next layer wrapping localchain and vtransfer powers, but in an attenuated fashion.  They don't introduce more powers than are already available via `LOA.monitorTransfers(...)`.

### Scaling Considerations

More message exchanges, but most of those are caused by inbound Cosmos transactions, so they should not consume much CPU per block.

### Documentation Considerations

New API surface.  Existing data and deployments will not be affected, especially since the orchestration vat+API has not yet landed on chain.

### Testing Considerations

Unit and bootstrap tests have been implemented.  The `agoric-sdk/packages/boot/test/supports.ts` fake bridge has been reorganised to greater mimic the actual chain and make tests pass.

### Upgrade Considerations

Some more Exos shipped with the Orch API, with the usual considerations of adding new Exo singletons and classKits and maintaining them.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants