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

Implement transaction pathing in 'aragon apm publish' #425

Merged
merged 3 commits into from Apr 26, 2019

Conversation

Projects
None yet
4 participants
@izqui
Copy link
Member

commented Apr 3, 2019

Requires aragon/aragon.js#273 and aragon/apm.js#35

Makes aragon apm publish use the new publishVersionIntent from apm.js which returns an intent object that is then used to do transaction pathing using an execTask.

By having transaction pathing for publishing we will be able to have governance in aragonPM registries, which will be needed for decentralizing aragonpm.eth (see AGP draft).

@izqui izqui requested review from 0xGabi and 0x6431346e Apr 3, 2019

@0xGabi

This comment has been minimized.

Copy link
Member

commented Apr 4, 2019

I tested locally publishing a vault app and is working.

Relevant output:

aragon apm publish minor --debug
...
Publish vault.aragonpm.eth [started]
Generating transaction [started]
→ Fetching DAO at 0x983b4Df4E8458D56CFDC51B9d2149381AF80308A...
Generating transaction [completed]
Sending transaction [started]
→ Waiting for transaction to be mined...
Sending transaction [completed]
Publish vault.aragonpm.eth [completed]
...

@izqui Make sense to further test with a more complex DAO structure like the one described in the AGP draft?

@0x6431346e

This comment has been minimized.

Copy link
Member

commented Apr 6, 2019

It seems to break when deploying to open.aragonpm.eth on rinkeby:

No ABI specified in artifact for 0xECbf3DE8ba9852Db55EA6Fd2Dae41FEAEbAE3Dd8

I looked into it and it seems to be because getAPMAppInfo only works for apm-repo, see https://github.com/aragon/aragon.js/pull/273/files?file-filters%5B%5D=.js#diff-47c422702e70cd7374a8a1a405acb0e0R15 .

Is there a use case for deploying apm apps to open.aragonpm.eth?

PS C:\Users\daniel> aragon dao apps 0x2DF31C5fcBB94656fc98e2953cF0DA03e2885Cfa --environment aragon:rinkeby
 √ Inspecting DAO
 √ Successfully fetched DAO apps for 0x2DF31C5fcBB94656fc98e2953cF0DA03e2885Cfa
┌───────────────────┬────────────────────────────────────────────┬───────────────────┐
│ App               │ Proxy address                              │ Content           │
├───────────────────┼────────────────────────────────────────────┼───────────────────┤
│ kernel            │ 0x2DF31C5fcBB94656fc98e2953cF0DA03e2885Cfa │ (No UI available) │
├───────────────────┼────────────────────────────────────────────┼───────────────────┤
│ acl               │ 0x0549c34e7c5d10a98325ad06ecae465e79b1f845 │ (No UI available) │
├───────────────────┼────────────────────────────────────────────┼───────────────────┤
│ evmreg            │ 0x513df522ff7f9dddd414dc0af6ae92f91a7170d8 │ (No UI available) │
├───────────────────┼────────────────────────────────────────────┼───────────────────┤
│ apm-enssub.open   │ 0x2f8b78369ab83ba1db5bf6d9755407abe24d4a18 │ ipfs:enssub       │
├───────────────────┼────────────────────────────────────────────┼───────────────────┤
│ apm-registry.open │ 0x915c4a47e7c7f7ab04ac70b4bcfba257a1e8b040 │ ipfs:apm          │
├───────────────────┼────────────────────────────────────────────┼───────────────────┤
│ apm-repo.open     │ 0xb61dffb7b5b4340691d8e8e64327e7d8525dfd7e │ ipfs:repo         │

Perhaps we should not use apm-repo.open at all?

To keep backwards compatibility we would have to add them to APM_APP_MAPPINGS. 🤔

@sohkai

This comment has been minimized.

Copy link
Member

commented Apr 9, 2019

Is there a use case for deploying apm apps to open.aragonpm.eth?

I don't think so... we probably deployed them by accident because of our automated scripts, but there's not really a good reason to recursively create these "apm-native" repos. I'd say we shouldn't use them at all.

@0xGabi 0xGabi assigned 0xGabi and unassigned 0xGabi Apr 10, 2019

@0x6431346e 0x6431346e added this to the aragonCLI v5.7 milestone Apr 24, 2019

@0x6431346e

This comment has been minimized.

Copy link
Member

commented Apr 26, 2019

Adding suggestions from code review & rebased & tested again. Looks good!

We can merge this as soon as new versions of @aragon/apm and @aragon/wrapper are published.

@0x6431346e 0x6431346e force-pushed the publish-pathing branch from 51479a1 to 79f5b99 Apr 26, 2019

@0xGabi

0xGabi approved these changes Apr 26, 2019

@0xGabi 0xGabi changed the base branch from master to live-dao-installs Apr 26, 2019

@0xGabi 0xGabi changed the base branch from live-dao-installs to master Apr 26, 2019

@0xGabi 0xGabi merged commit 7c36299 into master Apr 26, 2019

7 checks passed

build build
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
install install
Details
license/cla Contributor License Agreement is signed.
Details
lint lint
Details
test test
Details

@0xGabi 0xGabi deleted the publish-pathing branch Apr 26, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.