Add events to PoPA contract

[fvictorio]

Part of the audit findings Closes #125.

Add events to PoPA contract.

[fvictorio]

I didn't add tests for this, because testing events with ganache is very unreliable (sometimes they work, sometimes they don't). If someone knows how to do it properly, please let me know.

[coveralls]

Pull Request Test Coverage Report for Build 409

• 7 of 7 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.09%) to 79.135%

---

Totals
Change from base Build 405:	0.09%
Covered Lines:	948
Relevant Lines:	1173

---

💛 - Coveralls

[phahulin]

My suggestions:

1. drop Log prefix for names of events, e.g. AddressConfirmed
2. add a function userAddressByKeccakIdentifier() analogous to other userAddressBy... functions and make AddressRegistered, AddressUnregistered have the same signature as AddressConfirmed(address wallet, bytes32 keccakIdentifier)

[fvictorio]

1. I added the prefix because it's recommended here (and I agree with the idea). But I'm OK with removing it.
2. I agree, I'll make that change.

[fvictorio]

@phahulin I updated the event for address confirmation. I didn't have to add a new method, since I had the index of the physical address for that wallet.

It bothers me that the LogAddressRegistered event has string name, but the other two don't. And there doesn't seem to be an easy way to obtain it. I see two options here:

1. We could remove the name from that event, so that it's consistent with the other two.
2. We could add a method to get the name for a given physical address, but it seems like a waste of gas.

Also, do you think we should add indexed to the wallets in the events?

[garatortiz]

@fvictorio there are conflicts.

[fvictorio]

Fixed.

I had to inline the first argument to signerIsValid in the confirmAddress method because I was getting another "Stack too deep" error. Which is really weird, since in a previous commit I had to extract an inlined expression (in userAddressInfo) to fix that error.

I guess in one case it was caused by having too many local variables and in the other by having too much nesting, but I'm not really sure. If someone knows better, please let me know.

[phahulin]

@fvictorio

1. what if we go the other way - we only emit wallet and keccakIdentifier everywhere, but add a function that returns address by keccakIdentifier?
2. yes, I think we should add indexed on wallet

[fvictorio]

Oh, that's what you were saying in your previous comment? Sorry, I got it completely backwards. I think that should work. After all, with the event and the contract you should be able to get all the relevant information.

[fvictorio]

@phahulin I just added those changes.

Regarding the indexed, I didn't add it to the LogSignerChanged and LogRegistryChanged events, it didn't seem useful there.