[WIP] Multi identity

[maxsam4]

Trying different approaches.
Tests not updated.
Do not merge.

[adamdossa]

Some thoughts (I realise this is only a PoC, so some of the below may have been intentionally ignored for this code):

• IMO data storage and keyrings should be split out into separate contracts. This would allow us to have different implementations for these two bits of functionality and develop them somewhat independently (and help with the inevitable contract size issues ;-)).

e.g. interface for KeyRing contract is:
function addressToId(address) returns (uint256)
interface for
function getBool(uint256 _id, bytes32 _key) returns (bool)
etc.

a dummy implementation of KeyRing (which just supports single addresses per keyring) is then:
function addressToId(address _add) returns (uint256) {return uint256(_add)};

we could offer issuers different KeyRing implementations depending on their use-case (i.e. whether they want to allow multiple keys per identity, and the controls around that behaviour).

• I think data should be written directly to the data storage contract, rather than through modules, for things like identity. Modules then read that data and interpret it. Maybe we could have a new module type (DATA_KEY) which entitles a module to write to the storage as well if needed, and all writes to the IdentityStorage contract have to happen through the ST (ST has set functions with onlyOwnerOrModuleType(DATA_KEY)). These functions can also check that an address which is having data stored for it is correctly setup as a KeyRing before storing the data (and set it up if needed).

• Where to store meaningful keys - we want to be consistent with them across modules, but also want them to be flexible. Could keep a mapping / array of valid keys in the ST perhaps.

[maxsam4]

IMO data storage and keyrings should be split out into separate contracts. This would allow us to have different implementations for these two bits of functionality and develop them somewhat independently (and help with the inevitable contract size issues ;-)).

Makes sense, will incorporate in next Iteration.

I think data should be written directly to the data storage contract, rather than through modules, for things like identity. Modules then read that data and interpret it. Maybe we could have a new module type (DATA_KEY) which entitles a module to write to the storage as well if needed, and all writes to the IdentityStorage contract have to happen through the ST (ST has set functions with onlyOwnerOrModuleType(DATA_KEY)). These functions can also check that an address which is having data stored for it is correctly setup as a KeyRing before storing the data (and set it up if needed).

That's how I started building it out but STFactory went out of gas (its still out of gas even after removing the extra functions).

Maybe I should just change the permission check in IdentityStorage from onlyModule(Data_key) to onlyModule(Data_key) or owner or delegateWithPermission?

Where to store meaningful keys - we want to be consistent with them across modules, but also want them to be flexible. Could keep a mapping / array of valid keys in the ST perhaps."
I was thinking od defining a standard to follow. X key -> Y reason.

I thought about stoing keys onchain but i just couldn't find a proper use of them. Yeah, we can fetch what keys are valid but what's the use of that info to a smart contract?
If we define a standard and maybe a central contract where devs can register what keys they are going to use for what tasks, that will be more useful. I believe the keys should be global rather than ST specific.

[adamdossa]

Thanks. Contract size issues for the ST are a pain. Maybe this is something we need to try and tackle more directly in 3.0.0 although good options are quite limited.

Agree on the key side - can't think of an immediate reason why storing them on-chain is useful, other than to help with copy / paste type issues (i.e. have everyone reference them through consts on one of the contracts).

[adamdossa]

if associatedTokens.length == 0 then it seems like onlySecurityToken will always pass. Could move the require statement outside of the if clause.

[maxsam4]

If there are no associated tokens then by whom and why is this contract being used? :p
My original thinking was that the first security token that gets associated with this identity store should be free to do so and others will have to go through this modifier to be added. I'll look into a better flow to have this and ST associated and modify the logic accordingly.

[adamdossa]

Hey @maxsam4 - an interesting ERC which may be relevant for the IdentityStorage is:
ethereum/EIPs#1616
could you take a look and see if you think it is a more general way to do that part?

Next step is to do this fully in 3.0.0 - for now please can you just put in the IdentityStorage part rather than the KeyRing piece - on the KeyRing side it is bound up in the conversation around multi-sig / meta-transactions, so I think it is premature to add it now and we need to reach a firmer conclusion on the broader conversation first.

[maxsam4]

Thanks for the feedback @adamdossa. I'll look into the ERC.
I agree with you on splitting Identity storage and keyring. This was just an early prototype of what a keyring will look like. The code makes visualizing easier :).

I'll put the key ring development on hold and focus on research around multi-sigs vs key rings and meta tx.

I'll continue the implementation of identity storage. Perhaps after refactoring to solidity 0.5.0.

[maxsam4]

Closing this PR for now. Please don't delete the branch :)
I'll submit a new PR when there is something new to show.