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

Agent: add initial background script #952

Closed
wants to merge 20 commits into from

Conversation

sohkai
Copy link
Contributor

@sohkai sohkai commented Aug 27, 2019

Not yet tested, and has a few todos to complete (pending more work on aragon.js).

Passes to the frontend data in the form of:

{
  balances: [
    // This is the same as Finance's; all numerical values are strings meant
    // to be converted into normal numbers or BN.js objects on the frontend
    {
      address,
      amount,
      decimals,
      name,
      symbol,
      verified
    }
  ],
  transactions: [
    {
      id,  // <transactionHash>.<transactionIndex>.<logIndex>; not meant to be shown to humans
      date,  // Similar to Finance's; in milliseconds as a normal number
      description,  // string
      safe,  // boolean; only true if `SafeExecute()` was used (for later use)
      tokenTransfers: [
        {
          amount,  // string; BN number
          from,  // address of sender if transfer was to the Agent app, null if not
          to,  // address of the recipient if transfer was from the Agent app, null if not 
          token,  // token address
        }
      ],
      transactionHash, // string bytes64 hash for the transaction
      type, // string; see transaction-types.js (one of 'direct deposit', 'direct transfer', or 'contract interaction')
    }
  ],
}

The null possibility for the from and to fields of the tokenTransfers is intended to help front-end filtering based on incoming / outgoing transactions, but can be changed to suit the frontend as necessary.

Note that tokenTransfers is an array that can hold multiple token transactions (a single transaction that executes a contract can include many internal token transfers). For flexibility / simplicity, it currently does not do any grouping on the transfers, although we may want to do so in the frontend (sum all amounts for the same token) to meet the current designs.

There are a few edge cases in the current reducing that will require more complex detection to provide good UX. One example is a single transaction invoking multiple Agent executions that each transfer tokens. Another is directly receiving ETH onto the Agent's proxy from a contract interaction, as it's difficult to correlate receiving ETH with a contract execution based solely on the events in the general case. In the current state, these cases will be presented slightly confusingly to users (most likely symptom is duplicate / multiple transaction entries for a single data).


TODO:

  • Test, especially decoding token transfers and receiving ETH through contract interactions
  • Upgrade aragon.js to automatically include AppProxy events ABI (ProxyDeposit is not currently automatically decoded)
  • Upgrade aragon.js with API to fetch current app's contract address
  • Upgrade aragon.js with API for describing radspec
  • Add description fallback with check to function signature registry

@AquiGorka
Copy link
Contributor

AquiGorka commented Aug 29, 2019

Transactions has a tokenTransfers array which means each transaction can have multiple token transfers, the designs expect one amount (colored to refer to deposit/withdrawal):

image

What would be the best approach here?

Update: designs expect a token too, which token should be displayed here?

Update 2: same with Source/Recipient it expects one address

@sohkai
Copy link
Contributor Author

sohkai commented Aug 29, 2019

The best option might be to use the DataView's "entry expansion" to show multiple transactions.

If we don't want to do that now though, we could just say "multiple tokens" and "multiple" for source / recipient for the row.

@AquiGorka
Copy link
Contributor

How about this:

  • If there is only 1 tx use that data
  • If there's more use the word multiple as you suggested

@sohkai sohkai force-pushed the newstyle/agent-data branch 2 times, most recently from 78cf1aa to b988b3b Compare September 5, 2019 09:37
@coveralls
Copy link

Coverage Status

Coverage remained the same at 97.914% when pulling b988b3b on newstyle/agent-data into e5fa726 on newstyle/agent.

luisivan and others added 18 commits September 9, 2019 14:22
* Updated Tokens and Finance screenshots to 0.8

* Optimize new assets

* Added voting screenshots

* Optimize voting assets
* Small typo fix in Voting

* Changed vote action copy
* Agent: add boilerplate based on Finance

* Agent: update manifest to point to UI and script files

* Agent: add missing rxjs dependency

* Agent: update web3 dependencies to 1.x versions

* Reducer: update transactions reducer

* FrameModal: remove duplicate close button

* Transfer: update rendering according to new mocked data

* Update mock data

* Transfer: improve spacing on inner entry

* Update deps versions

* Remove unused components & assets

* Reducer: update metadata on transactions about incoming & outcoming token transfers

* Update mocked data

* useFilteredTransfers: add independent hook

* Update to use new useFilteredTransfers hook

* Remove unused asset

* useDownloadData: add hook to abstract logic

* Transfers: move download logic to hook

* Set format date format

* Transfers: remove unused imports and remove above & below

* useFilteredTransfers: remove unused imports

* Transfers: remove unused import

* useDownloadData: update imports

* Transfers: simplify component and remoe unused function

* InstallFrame: no need to check prop types if there are no props

* FrameModal: update dependencies and add href for Frame info

* FrameSvg: fix lint error

* FrameAndAgentSvg: fix lint error

* App: remove useViewport and unused imports

* Remove unused file

* Agent: update dependencies

* Agent: add token ABIs

* chore: add build task in main agent repo

* Changed install Frame copy

* Add agent svg

* AgentSvg: add component to access svg

* ComingSoon: add component

* App: render coming soon instead of balances & transfers

* InstallFrame: hide info box if Frame is running

* ComingSoon: update copy

Co-Authored-By: Jorge Izquierdo <jorge@aragon.one>

* ComingSoon: update copy

Co-Authored-By: Jorge Izquierdo <jorge@aragon.one>

* ComingSoon: update copy

Co-Authored-By: Jorge Izquierdo <jorge@aragon.one>
* Small typo fix in Voting

* Changed vote action copy

* Change voting action copy
* Small typo fix in Voting

* Changed vote action copy

* Change voting action copy

* Fixes suggestions
* Latest icons

* svgoify all the things
// TODO: add call to fetch current agent app's address once available in aragon.js
// and add it to settings
app
.call('initialized')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.call('initialized')
.call('hasInitialized')

.then(() => initialize())
.catch(_ => {
throw new Error(
'Could not start background script execution due to the contract not being connected to a network:'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'Could not start background script execution due to the contract not being connected to a network:'
'Could not start background script execution due to the contract not being connected to a network'

@sohkai
Copy link
Contributor Author

sohkai commented Mar 11, 2020

Closing for #1082

@sohkai sohkai closed this Mar 11, 2020
@sohkai sohkai deleted the newstyle/agent-data branch March 11, 2020 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants