Feature manifest and permission system #766
Conversation
shargon
commented
May 20, 2019
•
shargon
commented
- Close Feature manifest and permission system for NeoContract #287
- Close Store contract ABI on the blockchain #343
|
Can a contract update permissions without migrating to a new contract? |
Yes. |
| return true; | ||
| } | ||
|
|
||
| if (manifest.Trusts != null && !manifest.Trusts.IsWildcard && !manifest.Trusts.Contains(Hash)) |
There was a problem hiding this comment.
trusts is for displaying warnings by client or wallet only.
There was a problem hiding this comment.
Then trusts and safeMethods is only for user interfaces?
|
@vncoelho do you want to review it again? |
|
With pleasure, in the middle of it, in 2 hours it will done. |
| } | ||
| else if (Contract.IsGroup) | ||
| { | ||
| if (manifest.Groups.All(p => !p.PubKey.Equals(Contract.Group))) return false; |
There was a problem hiding this comment.
Does All work as expected or Sum would also work?
There was a problem hiding this comment.
It is checking, for all groups, if any of them is not equal.
Maybe this is strange, because one of yours groups may not allow you to enter, but another one can allow you to interact with the desired contract.
There was a problem hiding this comment.
If p.PubKey.Equals(Contract.Group) is false for any of the groups of have it will probably return false, no?
There was a problem hiding this comment.
If p.PubKey.Equals(Contract.Group) is false for ALL of the groups, it will return false.
| [TestMethod] | ||
| public void ParseFromJson_Groups() | ||
| { | ||
| var json = @"{""groups"":[{""pubKey"":""03b209fd4f53a7170ea4444e0cb0a6bb6a53c2bd016926989cf85f9b0fba17a70c"",""signature"":""41414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141""}],""features"":{""storage"":false,""payable"":false},""abi"":{""hash"":""0x0000000000000000000000000000000000000000"",""entryPoint"":{""name"":""Main"",""parameters"":[{""name"":""operation"",""type"":""String""},{""name"":""args"",""type"":""Array""}],""returnType"":""Any""},""methods"":[],""events"":[]},""permissions"":[{""contract"":""*"",""methods"":""*""}],""trusts"":[],""safeMethods"":[]}"; |
There was a problem hiding this comment.
How does these groups really work? A contract can be part of more than one group in this current design, right?
And why the signature?
There was a problem hiding this comment.
Without signature, you can join any group, which is not by design.
There was a problem hiding this comment.
Ok, I got it, in the current design, how is able to sign for entering? Is it a multisig with 1 single threshold?
There was a problem hiding this comment.
Just put the signature in the json.
There was a problem hiding this comment.
The group identifier is the PublicKey, right? 03b209fd4f53a7170ea4444e0cb0a6bb6a53c2bd016926989cf85f9b0fba17a70c
Which signatures will be allowed?
If we have 10 members of the group, should we create a MultiSig 1/10? For each new member, append an additional PubKey?
Sorry for the trivial questions, @erikzhang, trying to get the idea here...aehauheau
There was a problem hiding this comment.
Please check the specification here: #287
Where pubkey represents the public key of the group and signature is the signature of the contract hash.
|
I am still finishing, but it would be great if we could wait for @igormcoelho, he is on a trip today. We may have to wait some couple of hours. |
There was a problem hiding this comment.
Interesting implementation. My only opinion is that two things could be different: (1) groups could be a smart contract perhaps native (some NEP that allows or denies contracts to be invoked). But I agree that pubkey sig system will work.
(2) there could be an option for "final" contracts, that cannot be migrated. This would also apply for native contracts, that evolve outside the scope of migration system.
|
Something that may benefit users a lot is a DeployGroup operation, that deploys all group contracts at once (for the price of features + number of contracts). This could be cheaper and avoid people wanting to embed all in one just for price. |
If you remove the call of |
