git-consensus/contracts

🎨 Refactor: Merge hash->address mappings into one, consolidate getter methods

Closed this issue · 1 comments

Having two mappings is actually completely unnecessary:

mapping(bytes20 => address) private commitToOwnerAddr;
mapping(bytes20 => address) private tagToTokenAddr;

because no SHA-1 hash from CommitData could ever overlap with TagData.

Can simplify the GitConensus interface from:

addCommit(CommitData commit) 
addRelease(TagData tag, bytes20[] hashes, uint256[] values) 
commitAddr(bytes20 commitHash) 
commitExists(bytes20 commitHash) 
tagAddr(bytes20 tagHash) 
tagExists(bytes20 tagHash)

to

addCommit(CommitData commit) 
addRelease(TagData tag, bytes20[] hashes, uint256[] values) 
hashAddr(bytes20 hash) 
hashExists(bytes20 hash) 

It can be argued that the former gives more information about the type of hash (commit vs. tag), but this does not seem worth the trade-off of having a more complex interface.

Yeah, the simpler interface is better and makes sense.