git-consensus / contracts

Converts the informal ownership structure of an open-source git project to a formal DAO, with token distribution mechanisms for contributors.
GNU General Public License v3.0
13 stars 0 forks source link

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

Closed mattstam closed 2 years ago

mattstam commented 2 years ago

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 2 years ago

Yeah, the simpler interface is better and makes sense.