-
Notifications
You must be signed in to change notification settings - Fork 3
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
Port aragonPM from aragonOS #1
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
I was debating about moving everything over exactly as it were when we deployed aragonPM last (so on aragonOS v4.0.0), so we could create a tag and deployment log, and then apply the rest of the updates here.
I think this would just involve splitting up this PR into two parts, which may not be so difficult, but if it turns out to be, let's skip it.
test/contracts/ens/ens_subdomains.js
Outdated
}) | ||
|
||
it('fails if initializing without rootnode ownership', async () => { | ||
const ens = await ENS.new() | ||
const newRegProxy = await AppProxyUpgradeable.new(dao.address, hash('apm-enssub.aragonpm.eth'), EMPTY_BYTES) | ||
const newReg = ENSSubdomainRegistrar.at(newRegProxy.address) | ||
const newRegProxy = await AppProxyUpgradeable.new(dao.address, hash('apm-enssub.aragonpm.eth'), '0x') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, we may want to play with remix or debug this a bit more.
Theoretically 0x00
should be a bytes array of length 0, but it looks like it may not be interpreted correctly in AppProxyBase
.
c04dcf4
to
ef1ba63
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Last thing: let's also copy the LICENSE.md from aragonOS!
I recommend reviewing per commit
@sohkai it would be nice to add this repo to Travis