Agoric / agoric-sdk

monorepo for the Agoric Javascript smart contract platform
Apache License 2.0
325 stars 205 forks source link

orchestration - ICA Controller - Account - vstorage logging #9066

Open ivanlei opened 6 months ago

ivanlei commented 6 months ago

What is the Problem Being Solved?

https://github.com/Agoric/agoric-sdk/issues/9065 details the ability to send ICA commands over an ICA account object.

This enhances that capability by logging well-structured information to a subtree of the contract's storage node that allows for following TX progress and debugging. This universal logging brings uniformity to error handling, debuggability, & TX recovery.

Description of the Design

current draft:

Security Considerations

Scaling Considerations

Test Plan

Upgrade Considerations

rowgraus commented 5 months ago

Questions coming out of planning:

  1. What actions should be automatically stored?
  2. How do we want developers to specify additional information
  3. What information do users require in order to track their own transaction
  4. First user story
LuqiPan commented 4 months ago

From security's perspective, should we be worried about this being another factor that contributes to vstorage(hence disk) growth?

turadg commented 4 months ago

contributes to vstorage(hence disk) growth?

A concern, yes. It should not be exploitable because it's managed by the Orchestration API. The API can also delete old data so it doesn't persist in IAVL.

dtribble commented 4 months ago

contributes to vstorage(hence disk) growth?

A concern, yes. It should not be exploitable because it's managed by the Orchestration API. The API can also delete old data so it doesn't persist in IAVL.

Indeed that's a motivator for having system-provided txn logging with sufficient info for most purposes

dckc commented 3 months ago
  • each asyncFlow/orchestration has an ID, which is used as a root for vstorage records of progress

Is the id scoped to the contract? (in particular: to the orchestration facade?)

It's not supposed to be global, right?

dckc commented 3 months ago
  • there is a record associating the async flow with the walletSpendAction

It's straightforward to point from a wallet's vstorage to a key of the contract's choosing (using offerToPublicSubscriberPaths).

Going the other way doesn't seem feasible. We don't need to go the other way, do we?

0xpatrickdev commented 2 months ago

When an ICA message lands on another chain, or any cross-chain ibc message for that matter, the following transaction shows up: MsgRecvPacket

On MsgRecvPacket, we have Packet.

Notably, the primary way to filter transactions on a remote chain is by using the following fields:

// identifies the port on the sending chain.
string source_port = 2;
// identifies the channel end on the sending chain.
string source_channel = 3;
// identifies the port on the receiving chain.
string destination_port = 4;
// identifies the channel end on the receiving chain.
string destination_channel = 5;

This seems to suggest we want to make these values available in vstroage, in addition to the account address we are currently publishing.

0xpatrickdev commented 2 months ago

Until #9066, every interchain action has been triggered by a wallet offer. This meant that the actions taken by the LCA were clearly documented in published.wallet. For scenarios where external triggers initiate interchain transactions, it seems it would be helpful to log something this vstorage to figure out something happened. This might suggest we want do vstorage writes when tx's are completed (and initiated).

dckc commented 1 month ago

Until #9066, every interchain action has been triggered by a wallet offer. This meant that the actions taken by the LCA were clearly documented in published.wallet. ...

There's a pattern where, for example, the vaultFactory publishes info about vaults and the wallet points to those parts of vstorage by path. It's used in watchUserVaults in dapp-inter.

Likewise, the account / tx info could be canonically published by the contract, and the wallet could get a pointer.

0xpatrickdev commented 1 month ago

Notes from talking with @michaelfig -

image