FirstIdentityWallet Public Key is not stored

[VictorNS69]

As shown in the images, the public key of the first identity created when deploying, is not storing in the public key registry Smart Contract.

I think the issue is that the initialize function in the PublicKeyRegistry must have 2 more arguments, "proxyAddress" and "publicKey", so when we initialize for the first time the contracts, we can pass as an arguments both data.

This change will also imply changing the migrations file (4_initialize_contracts.js)

[xdaniortega]

Hi! I was having a look at this bug and the solution proposed above looks like this: function initialize(address _previousPublishedVersion, address _proxyAddress, bytes32 _publicKey) initializer public{ version = 4; previousPublishedVersion = _previousPublishedVersion; addKey(_publicKey); } But I cannot figure out why we need the proxyAddress.

[VictorNS69]

Hi @DaniOrtegaB .

The addKey() function uses the msg.sender as the subject of the public key being added.

The problem here is that when we initialize the contract, the msg.sender is not the proxy address, so I think we need to add the proxyAddressargument the same way we did with the AlastriaNameService (ANS migrations)

The final goal is to add the public key of the fistIdentityWallet proxy (0x61f4...), not it's EOA (0x64326...)

[xdaniortega]

Hii @VictorNS69 ,

Ok! So basically I added a new definition for addKey function wich takes 2 arguments (pk, proxyAddrres) and changed the references to msg.sender to the address of the proxy.

I update the other calls for the function while I wait your response :)

[VictorNS69]

Hi @DaniOrtegaB sorry for the delay.

If you think you have fixed this issue, please create a PR and add @nefera606 and me as reviewers.

Thanks!

[xdaniortega]

Hii, today I was thinking about other ways of solving this problem. If our goal is to create an identity, I'd rather save its Pk and address calling the public key registry in the initialize function of the AlastriaIdentityManager contract, instead of in the initalize funtion in the PublicKeyRegistry.

By doing it in that way, while initializing a new entity we'll have its address saved without having to initialize again a publicKeyRegistry or calling any addKey method.

[VictorNS69]

It can be a good idea I think.

Let's hear what @nefera606 thinks.

[xdaniortega]

Hi @VictorNS69 ! The solution I found achieves the following:

With that solution, we could close #36 but I found some important variables to review. In some contracts Pk are defined as string memory and in other as adresses: (Ex. At identityManager _firstIdentityWallet and firstIdentityWallet are both defined as address but in PublicKeyRegistry, we have the identityWallet address as address and the Pk as string memory)

So to deal with that I had to convert address to string memory with: function toAsciiString(address x) internal view returns (string memory) { bytes memory s = new bytes(40); for (uint i = 0; i < 20; i++) { bytes1 b = bytes1(uint8(uint(uint160(x)) / (2*(8(19 - i))))); bytes1 hi = bytes1(uint8(b) / 16); bytes1 lo = bytes1(uint8(b) - 16 uint8(hi)); s[2i] = char(hi); s[2*i+1] = char(lo);
} return string(s); }

function char(bytes1 b) internal view returns (bytes1 c) {
    if (uint8(b) < 10) return bytes1(uint8(b) + 0x30);
    else return bytes1(uint8(b) + 0x57);
}

This will increase SC deploy, so maybe is better to define Pk as bytes32 variables. Btw, If anyone can also check if that Pk is correct It'd be awesome :)

[VictorNS69]

Hi @DaniOrtegaB in the last meeting, @nefera606 said he already solved this, and that he can easily push his code.

@nefera606 are you able to push your code soon? Maybe, creating a new PR?

[nefera606]

Hello, I have been reviewing the topic and I have some points:

• Is the public key of this first identity completely necessary? I mean this first identity acts as the identityManager admin (despite being the admin of the update proxies) so, this account may never generate any credentials. I have the code to add this key, but now I am thinking that it is not correct. We can discuss this at the next meeting.
• The public key should be a string because of the possibility of not using the ES256K algorithm for the JWT signature.
• firstIdentityWallet is the address of the proxy contract, not the public key.