🎨 Refactor: Merge hash->address mappings into one, consolidate getter methods
Closed this issue · 1 comments
mattstam commented
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.
DmitriyShepelev commented
Yeah, the simpler interface is better and makes sense.