(Refactor / Improvement) Using the keccak for comparing inputs

[Janther]

On the lookup by physical address

if (
       str_eq(users[wallet].physical_addresses[ai].country, country)
    && str_eq(users[wallet].physical_addresses[ai].state, state)
    && str_eq(users[wallet].physical_addresses[ai].city, city)
    && str_eq(users[wallet].physical_addresses[ai].location, location)
    && str_eq(users[wallet].physical_addresses[ai].zip, zip)
)

Instead of looping over every character on the string comparison for every input, the system could benefit from hashing the keccak of the attributes when storing it and using it for comparison.

if (users[wallet].physical_addresses[ai].keccak == keccak256(country, state, city, location, zip))

The keccak could also be used as an index or a proof of the existence of an address.

mapping (bytes32 => bool) addressExists;

[phahulin]

@Janther could you elaborate on

keccak could also be used as an index or a proof of the existence of an address.

At the present moment we store addresses for each wallet as an array - this is probably not the best way. But the advantage of this is that we can find all addresses linked to the wallet. Do you propose we store an additional mapping of keccak to index in addresses array?

[Janther]

Yes mainly something like this:

contract ProofOfPhysicalAddress {
    mapping (bytes32 => bool) public addressExists;

    function register_address(
        string name,
        string country,
        string state,
        string city, string location, string zip,
        uint256 price_wei,
        bytes32 confirmation_code_sha3, uint8 sig_v, bytes32 sig_r, bytes32 sig_s)
    public payable
    {
        // ... normal register_address logic plus following line ... //
        addressExists[keccak256(name, country, state, city, location, zip)] = true;
    }
}

Then you can easily ask if an address has been registered. This is just an idea since the important part here is that we agree that keccak256(name, country, state, city, location, zip) is a proper way to create a unique identifier for an address so we can evaluate or use it as an index for mappings that would serve as shortcuts.

[phahulin]

Yes, we can agree that using keccak256 as a unique identifier and for searching should be done.

I'm trying to understand if there is any benefit to store both array of addresses and an additional mapping of addresses. But this discussion should probably go to https://github.com/poanetwork/poa-popa/issues/79

[Janther]

In the case of having a mapping of Users containing an array of addresses, it makes sense to have a mapping of booleans just to show that an address does exist without having to go through the Users. If we consider #79 then it makes no sense since the addresses are already accessible with a single call.

[phahulin]

@pablofullana can I start working on the part that me and @Janther agreed on? that is

Instead of looping over every character on the string comparison for every input, the system could benefit from hashing the keccak of the attributes when storing it and using it for comparison.

I think it will improve the code to a large extent.

[pablofullana]

Works for me. @vbaranov any concerns on your side?

[vbaranov]

no, I have no concerns about it