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

API changes: Intents, Forwarding Paths and Transactions #132

Open
1 task done
bpierre opened this issue Jul 11, 2020 · 1 comment
Open
1 task done

API changes: Intents, Forwarding Paths and Transactions #132

bpierre opened this issue Jul 11, 2020 · 1 comment

Comments

@bpierre
Copy link
Contributor

bpierre commented Jul 11, 2020

This issue will keep track of the planned changes regarding the intents, forwarding paths and transactions.

These changes are aiming to improve / solve the following:

  1. Separate the forwarding paths from the transactions to sign [1].
  2. Provide a standard transaction request object that can be passed to Ethereum libraries.
  3. Make it easier to sign a single transaction.
  4. Make it possible to ignore the forwarding paths.
  5. Make it possible to express complex forwarding paths.

[1] This is why the term “Forwarding Path” is now preferred to “Transaction Path”.

Current Status

  • Store the signing address in the context.

Going forward we'll use a milestone to keep track of the progress: https://github.com/aragon/connect/milestone/1

Storing the signing address in the connection context

The idea here is to attach a default address to the connection context. This address will get used by the methods generating forwarding paths and transaction requests. It will be possible to override it at any point.

It could be passed to connect():

const myorg = await connect('myorg.aragonid.eth', 'thegraph', {
  actAs: '0x…',
})

It should also be possible to update it without resetting the connection. A new method on Organization could help doing that:

org.actAs('0x…')

Reshaping the intent-to-transaction flow

ForwardingPath

  • Replaces AppIntent and TransactionPath.
  • Represents the forwarding path corresponding to an action that will be executed.
  • Returned by some methods of Organization and App (e.g. App#exec()).
  • Users should not instantiate it directly (using new).
class ForwardingPath {
  // A list of transaction requests ready to get signed.
  transactions: Transaction[]

  // Lets consumers pass a callback to sign any number of transactions.
  // This is similar to calling transactions() and using a loop, but shorter.
  // It returns the value returned by the library, usually a transaction receipt.
  sign<Receipt>(
    callback: (tx: Transaction) => Promise<Receipt>
  ): Promise<Receipt[]>

  // Return a description of the forwarding path, to be rendered.
  describe(): Promise<ForwardingPathDescription>

  // Return a description of the forwarding path, as text.
  // Shorthand for .describe().toString()
  toString(): string
}

New App methods: exec() and execPaths()

The two initial methods returning ForwardingPath instances are App#exec() (to get the shortest path) and App#execPaths() (to get all the possible paths).

type ExecOptions = {
  // The account to sign the transactions with. It is optional
  // when `actAs` has been set with the connection. If not,
  // the address has to be passed.
  actAs: Address

  // Optionally declare a forwarding path. When not specified,
  // the shortest path is used instead.
  path: ForwardingPathDeclaration
}

// No `path` option here since we want them all.
type ExecPathsOptions = {
  actAs: Address
}

interface App {
  exec(signature, params, options?: ExecOptions)
  execPaths(signature, params, options?: ExecPathsOptions)
}

Address

This is a type we could use for documentation purposes.

type Address = string

AppOrAddress

This type accepts an App instance or its address, and should get used whenever possible rather than an address only.

type AppOrAddress = App | Address

ForwardingPathDeclaration

This type allows to express a forwarding path in a simplified, but limited way: it is not possible to nest the forwarding actions.

type ForwardingPathDeclaration = AppOrAddress[]

Transaction

Transaction replaces TransactionRequest, and is now a type rather than a class. It describes a subset of the eth_sendTransaction parameters in the Ethereum JSON-RPC API.

type Transaction = {
  data: string
  from: Address
  to: Address
}

ForwardingPathDescription

This object contains all the information needed to render the description of a forwarding path. It gets returned by ForwardingPath#describe().

type ForwardingPathDescriptionTreeEntry =
  | AppOrAddress
  | [AppOrAddress, ForwardingPathDescriptionEntry[]]

type ForwardingPathDescriptionTree = ForwardingPathDescriptionEntry[]

class ForwardingPathDescription {
  // Return a tree that can get used to render the path.
  tree(): ForwardingPathDescriptionTree

  // Renders the forwarding path description as text
  toString(): string

  // TBD: a utility that makes it easy to render the tree,
  // e.g. as a nested list in HTML or React.
  reduce(callback: Function): any
}

Forwarding Path Builder

Forwarding Path Declaration Syntax

Paths can be complex and defining them using JS structures might not provide a level of clarity that is sufficient. Having a dedicated syntax could improve that.

Using tagged templates could make it possible to integrate our existing object types into it, App and Intent in particular.

Initial draft:

const p = path`
  > ${voting}
    > ?
      > ${voting.createVote('something')}
      > "encoded_sub_action_1"
      > "encoded_sub_action_2"
  > ${tokens}
    > "encoded_action_1"
      > "encoded_sub_action_1"
`

const txRequests = p.transactions({ as: '0x…' })

In this example, ? would express a part that need to be filled by the library, if possible.

Note: this draft is only used to express the idea, its syntax will probably be different.

Path Builder Utility

Using the language previously mentioned above, we could also provide a forwarding path builder. This tool would let users edit a forwarding path after its initial creation.

We could imagine an initial implementation accepting the same JS structure than the one returned by path\``.

Initial draft:

const pathBuilder = path`
  > ${voting}
    > ?
      > ${voting.createVote('something')}
      > "encoded_sub_action_1"
      > "encoded_sub_action_2"
  > ${tokens}
    > "encoded_action_1"
      > "encoded_sub_action_1"
`

// Call insert() in this way to insert an action after "encoded_sub_action_2"
pathBuilder.insert('0 > 0', voting.createVote('something'))

// PathBuilder could inherit from Intent since it would provide the same methods:
pathBuilder.sign()
pathBuilder.transactions()
pathBuilder.path()
pathBuilder.paths()

Note: this draft is only used to express the idea, its syntax will probably be different.

@0xGabi
Copy link
Contributor

0xGabi commented Jul 14, 2020

Woow, thanks a lot for doing this write-up. It makes things so much clear and easy to go ahead and be more confident in what we need to implement 🙏

First a few small nitpicks:

  1. On the Intent section. I think the example is doing await org.appIntent but if I understood correctly now that should be await org.intent.
  2. On the ForwardingPathDescription section. When you explain the types, maybe the ForwardingPathDescriptionTreeEntry should be ForwardingPathDescriptionEntry? Or I'm missing something there 🤔

I would like to start working on a first draft of the second item on the TODO list 🙋‍♂️

Finally would like if we could further sync on the 3rd and 4th items. Would like us to be a bit more specific on these ones. For example, what are the missing pieces until we consider we can check: "Finalize the path builder API".

0xGabi added a commit that referenced this issue Dec 30, 2020
This PR includes most of the changes discussed in #132 

We don't have an `Intent` object as it was an intermediary step that ends up adding more complexity to the code. Instead of giving an intent we calculate the path and then create a `ForwardingPath` object that provide extra functionality on top of the data:
- Sign transactions with a signer
- Describe the path using Radspec
- Apply pre-transactions to the path

On top of this PR there were a few changes to the app connectors for Agreements and Dandelion Voting
@0xGabi 0xGabi removed their assignment Jan 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants