poanetwork / poa-popa

DApp for proof of physical address (PoPA) attestation for validators of POA Network
https://popa.poa.network
GNU General Public License v3.0
24 stars 18 forks source link

(Improvement/refactor) Using the keccak256 to compare if an address matches a new entry. #84

Closed Janther closed 6 years ago

Janther commented 6 years ago

Before submitting a pull request, please provide the following information:

I performed a small refactor in the register_address function. Just cleaning up some duplicated code.

coveralls commented 6 years ago

Pull Request Test Coverage Report for Build 221


Changes Missing Coverage Covered Lines Changed/Added Lines %
blockchain/contracts/ProofOfPhysicalAddress.sol 13 16 81.25%
<!-- Total: 13 16 81.25% -->
Files with Coverage Reduction New Missed Lines %
blockchain/contracts/ProofOfPhysicalAddress.sol 14 35.53%
<!-- Total: 14 -->
Totals Coverage Status
Change from base Build 218: 0.6%
Covered Lines: 764
Relevant Lines: 1067

💛 - Coveralls
coveralls commented 6 years ago

Pull Request Test Coverage Report for Build 222


Changes Missing Coverage Covered Lines Changed/Added Lines %
blockchain/contracts/ProofOfPhysicalAddress.sol 14 18 77.78%
<!-- Total: 14 18 77.78% -->
Files with Coverage Reduction New Missed Lines %
blockchain/contracts/ProofOfPhysicalAddress.sol 1 35.29%
<!-- Total: 1 -->
Totals Coverage Status
Change from base Build 218: 0.6%
Covered Lines: 764
Relevant Lines: 1068

💛 - Coveralls
phahulin commented 6 years ago

@Janther should we also get rid of comparisons with empty strings in favour of keccak256 or it's not worth it?

// now
require(!str_eq(name, ''));

// maybe?
bytes32 empty_hash = keccak256('');
require( keccak256(name) != empty_hash );

and remove function str_eq altogether?

Janther commented 6 years ago

That's the plan for the next PR, I have the commit ready but I like to keep PRs atomic. Once this PR is merged, I'll make the next one.

And it will be like this:

// before
require(!str_eq(name, ''));

// after
require(bytes(name).length > 0);