Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

On-chain ACL checker for secretstore #5015

Merged
merged 48 commits into from Apr 3, 2017
Merged

Conversation

svyatonik
Copy link
Collaborator

on top of #4974
@keorn as agreed yesterday - here's the chain-based AclChecker prototype. Are you going to develop the contract itself? I'll need a contract ABI to generate something like this:
https://github.com/paritytech/parity/blob/master/ethcore/src/miner/service_transaction_checker.rs#L58
Also - are we going to deploy registry contract && get ACL checker contract address from there, or would it be hardcoded/cli-option address?

@gavofyork
Copy link
Contributor

would prefer to have the registry throughout.

@svyatonik
Copy link
Collaborator Author

Checked success && failure cases on Kovan

@svyatonik svyatonik added A0-pleasereview 🤓 Pull request needs code review. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Mar 27, 2017
@rphmeier rphmeier added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Apr 3, 2017
@rphmeier
Copy link
Contributor

rphmeier commented Apr 3, 2017

LGTM; I wonder if we could put the contract into the native_contracts crate at some point?

@svyatonik
Copy link
Collaborator Author

svyatonik commented Apr 3, 2017

I thought about it, but as I understand it is for 'core' functionality, while this one is secretstore-specific && secretstore itself is very specific. So do not know if this is a good idea.
Maybe it would be better to use macros from native_contracts to generate this file?

@gavofyork
Copy link
Contributor

i think it's fine to have in there. it's not a huge maintenance burden and it's super convenient.

@svyatonik
Copy link
Collaborator Author

ok, will move there

@svyatonik
Copy link
Collaborator Author

done

@gavofyork gavofyork merged commit abec06f into master Apr 3, 2017
@gavofyork gavofyork deleted the secretstore_aclstorage branch April 3, 2017 15:46
@rphmeier
Copy link
Contributor

rphmeier commented Apr 3, 2017

it is for 'core' functionality

I meant it to be a bin for all contracts we use from Parity (which I suppose includes this)

@svyatonik
Copy link
Collaborator Author

Ok, got it :) I thought that, because it doesn't include updater contract. But I suppose it is in TODO list.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants