From 698abd447deec687e45d31d1f770884e9cff55dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Wed, 10 Oct 2018 11:40:09 -0300 Subject: [PATCH 1/2] Removed dependency names can no longer be reused. --- packages/vouching/contracts/Vouching.sol | 13 ++++++++++--- packages/vouching/test/contracts/Vouching.test.js | 9 +++++++++ 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/packages/vouching/contracts/Vouching.sol b/packages/vouching/contracts/Vouching.sol index 8727b9220..95f39f622 100644 --- a/packages/vouching/contracts/Vouching.sol +++ b/packages/vouching/contracts/Vouching.sol @@ -23,6 +23,7 @@ contract Vouching is Initializable { using SafeERC20 for ERC20; struct Dependency { + bool used; address owner; address dependencyAddress; uint256 stake; @@ -63,9 +64,9 @@ contract Vouching is Initializable { require(initialStake >= _minimumStake, "Initial stake must be equal or greater than minimum stake"); require(owner != address(0), "Owner address cannot be zero"); require(dependencyAddress != address(0), "Dependency address cannot be zero"); - require(_registry[name].dependencyAddress == address(0), "Given dependency name is already registered"); + require(!_registry[name].used, "Given dependency name is already registered"); - _registry[name] = Dependency(owner, dependencyAddress, initialStake); + _registry[name] = Dependency(true, owner, dependencyAddress, initialStake); _token.safeTransferFrom(owner, this, initialStake); @@ -97,7 +98,13 @@ contract Vouching is Initializable { function remove(string name) external onlyDependencyOwner(name) { // Owner surrenders _minimumStake to the system uint256 reimbursedAmount = _registry[name].stake.sub(_minimumStake); - delete _registry[name]; + + // The 'used' field is not deleted, to mark this name as taken and + // prevent future dependencies from being created reusing it + delete _registry[name].owner; + delete _registry[name].dependencyAddress; + delete _registry[name].stake; + _token.safeTransfer(msg.sender, reimbursedAmount); emit DependencyRemoved(keccak256(abi.encodePacked(name))); } diff --git a/packages/vouching/test/contracts/Vouching.test.js b/packages/vouching/test/contracts/Vouching.test.js index d79d82ade..20a7e5ecb 100644 --- a/packages/vouching/test/contracts/Vouching.test.js +++ b/packages/vouching/test/contracts/Vouching.test.js @@ -288,6 +288,15 @@ contract('Vouching', function ([_, tokenOwner, vouchingOwner, developer, transfe amount.should.be.bignumber.equal(0); }); + it('reverts when a removed dependency\'s name is reutilized', async function () { + await this.vouching.remove(dependencyName, { from: developer }); + await assertRevert( + this.vouching.create( + dependencyName, developer, anotherDependencyAddress, stakeAmount, { from: developer } + ) + ); + }); + it('emits a DependencyRemoved event', async function () { const result = await this.vouching.remove(dependencyName, { from: developer }); assertEvent.inLogs(result.logs, 'DependencyRemoved', { From e3597cd8791c36017af4da80c2eff0265f83b7cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Wed, 10 Oct 2018 12:41:03 -0300 Subject: [PATCH 2/2] Moved the boolean outside of the Dependency struct. --- packages/vouching/contracts/Vouching.sol | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/packages/vouching/contracts/Vouching.sol b/packages/vouching/contracts/Vouching.sol index 95f39f622..9036ce3cf 100644 --- a/packages/vouching/contracts/Vouching.sol +++ b/packages/vouching/contracts/Vouching.sol @@ -23,13 +23,13 @@ contract Vouching is Initializable { using SafeERC20 for ERC20; struct Dependency { - bool used; address owner; address dependencyAddress; uint256 stake; } mapping (string => Dependency) private _registry; + mapping (string => bool) private _takenDependencyNames; uint256 private _minimumStake; ERC20 private _token; @@ -64,9 +64,10 @@ contract Vouching is Initializable { require(initialStake >= _minimumStake, "Initial stake must be equal or greater than minimum stake"); require(owner != address(0), "Owner address cannot be zero"); require(dependencyAddress != address(0), "Dependency address cannot be zero"); - require(!_registry[name].used, "Given dependency name is already registered"); + require(!_takenDependencyNames[name], "Given dependency name is already registered"); - _registry[name] = Dependency(true, owner, dependencyAddress, initialStake); + _takenDependencyNames[name] = true; + _registry[name] = Dependency(owner, dependencyAddress, initialStake); _token.safeTransferFrom(owner, this, initialStake); @@ -99,11 +100,9 @@ contract Vouching is Initializable { // Owner surrenders _minimumStake to the system uint256 reimbursedAmount = _registry[name].stake.sub(_minimumStake); - // The 'used' field is not deleted, to mark this name as taken and - // prevent future dependencies from being created reusing it - delete _registry[name].owner; - delete _registry[name].dependencyAddress; - delete _registry[name].stake; + // The entry is not removed from _takenDependencyNames, to prevent a new dependecy + // from reusing the same name + delete _registry[name]; _token.safeTransfer(msg.sender, reimbursedAmount); emit DependencyRemoved(keccak256(abi.encodePacked(name)));