feat(billboard): drop with custom treeId_

.gas-snapshot

@@ -70,22 +70,23 @@ CurationTest:testCannotCurateNativeTokenZeroAddress() (gas: 16488)
 CurationTest:testERC20Curation() (gas: 59908)
 CurationTest:testNativeTokenCuration() (gas: 60085)
 CurationTest:testNativeTokenCurationToContractAcceptor() (gas: 37466)
-DistributionTest:testCannotClaimIfAlreadyClaimed() (gas: 302159)
-DistributionTest:testCannotClaimIfInsufficientBalance() (gas: 411915)
-DistributionTest:testCannotClaimIfInvalidProof() (gas: 263211)
-DistributionTest:testCannotClaimIfInvalidTreeId() (gas: 261207)
-DistributionTest:testCannotDropByAttacker() (gas: 11092)
-DistributionTest:testCannotDropIfInsufficientAllowance(uint256) (runs: 256, μ: 230891, ~: 230907)
-DistributionTest:testCannotDropIfInsufficientBalance(uint256) (runs: 256, μ: 233150, ~: 233389)
-DistributionTest:testCannotDropIfZeroAmount() (gas: 150119)
-DistributionTest:testCannotSetAdminByAdmin() (gas: 17333)
-DistributionTest:testCannotSetAdminByAttacker() (gas: 11089)
-DistributionTest:testCannotSweepByAttacker() (gas: 246643)
-DistributionTest:testCannotSweepIfZeroBalance() (gas: 248436)
-DistributionTest:testClaim() (gas: 430983)
-DistributionTest:testDrop() (gas: 390240)
-DistributionTest:testSetAdmin() (gas: 20195)
-DistributionTest:testSweep() (gas: 268852)
+DistributionTest:testCannotClaimIfAlreadyClaimed() (gas: 286529)
+DistributionTest:testCannotClaimIfInsufficientBalance() (gas: 395958)
+DistributionTest:testCannotClaimIfInvalidProof() (gas: 246948)
+DistributionTest:testCannotClaimIfInvalidTreeId() (gas: 245026)
+DistributionTest:testCannotDropByAttacker() (gas: 11530)
+DistributionTest:testCannotDropIfInsufficientAllowance(uint256) (runs: 256, μ: 213977, ~: 213997)
+DistributionTest:testCannotDropIfInsufficientBalance(uint256) (runs: 256, μ: 216215, ~: 216454)
+DistributionTest:testCannotDropIfZeroAmount() (gas: 150488)
+DistributionTest:testCannotDropTwiceWithSameTreeId() (gas: 309084)
+DistributionTest:testCannotSetAdminByAdmin() (gas: 17334)
+DistributionTest:testCannotSetAdminByAttacker() (gas: 11134)
+DistributionTest:testCannotSweepByAttacker() (gas: 230371)
+DistributionTest:testCannotSweepIfZeroBalance() (gas: 232253)
+DistributionTest:testClaim() (gas: 416288)
+DistributionTest:testDrop() (gas: 374973)
+DistributionTest:testSetAdmin() (gas: 20239)
+DistributionTest:testSweep() (gas: 253199)
 LogbookNFTSVGTest:testTokenURI(uint8,uint8,uint16) (runs: 256, μ: 2019505, ~: 1310779)
 LogbookTest:testClaim() (gas: 135608)
 LogbookTest:testDonate(uint96) (runs: 256, μ: 155485, ~: 156936)

src/Billboard/Distribution.sol

@@ -5,29 +5,25 @@ import "@openzeppelin/contracts/utils/cryptography/MerkleProof.sol";
 import "@openzeppelin/contracts/access/Ownable.sol";
 import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
-import "@openzeppelin/contracts/utils/Counters.sol";
 
 import "./IDistribution.sol";
 
 // https://github.com/Uniswap/merkle-distributor
 contract Distribution is IDistribution, Ownable {
-    using Counters for Counters.Counter;
-    Counters.Counter public lastTreeId;
-
     address public admin;
     address public immutable token;
 
     // treeId_ => merkleRoot_
-    mapping(uint256 => bytes32) public merkleRoots;
+    mapping(string => bytes32) public merkleRoots;
 
     // treeId_ => balance_
-    mapping(uint256 => uint256) public balances;
+    mapping(string => uint256) public balances;
 
     // treeId_ => totalAmount_
-    mapping(uint256 => uint256) public totalAmounts;
+    mapping(string => uint256) public totalAmounts;
 
     // treeId_ => cid_ => account_
-    mapping(uint256 => mapping(string => mapping(address => bool))) public hasClaimed;
+    mapping(string => mapping(string => mapping(address => bool))) public hasClaimed;
 
     constructor(address token_, address admin_) {
         require(token_ != address(0), "Zero address");
@@ -60,12 +56,11 @@ contract Distribution is IDistribution, Ownable {
     //////////////////////////////
 
     /// @inheritdoc IDistribution
-    function drop(bytes32 merkleRoot_, uint256 amount_) external isFromAdmin returns (uint256 treeId_) {
+    function drop(string calldata treeId_, bytes32 merkleRoot_, uint256 amount_) external isFromAdmin {
         require(amount_ > 0, "Zero amount");
 
         // Set the merkle root
-        lastTreeId.increment();
-        treeId_ = lastTreeId.current();
+        require(merkleRoots[treeId_] == bytes32(0), "Existing tree");
         merkleRoots[treeId_] = merkleRoot_;
 
         emit Drop(treeId_, amount_);
@@ -80,7 +75,7 @@ contract Distribution is IDistribution, Ownable {
 
     /// @inheritdoc IDistribution
     function claim(
-        uint256 treeId_,
+        string calldata treeId_,
         string calldata cid_,
         address account_,
         uint256 share_,
@@ -110,7 +105,7 @@ contract Distribution is IDistribution, Ownable {
     }
 
     /// @inheritdoc IDistribution
-    function sweep(uint256 treeId_, address target_) external isFromAdmin {
+    function sweep(string calldata treeId_, address target_) external isFromAdmin {
         uint256 _balance = balances[treeId_];
 
         require(_balance > 0, "Zero balance");

src/Billboard/IDistribution.sol

@@ -8,7 +8,7 @@ interface IDistribution {
      * @param treeId_ Tree ID of the drop
      * @param amount_ Total amount of the drop
      */
-    event Drop(uint256 indexed treeId_, uint256 amount_);
+    event Drop(string indexed treeId_, uint256 amount_);
 
     /**
      * @dev Emitted when an claim is made.
@@ -39,12 +39,13 @@ interface IDistribution {
     /**
      * @notice Create a new drop
      *
+     * @param treeId_ Tree ID of new drop
      * @param merkleRoot_ Merkle root of new drop
      * @param amount_ Total amount of new drop
      *
      * Emits a {Drop} event on success.
      */
-    function drop(bytes32 merkleRoot_, uint256 amount_) external returns (uint256 treeId_);
+    function drop(string calldata treeId_, bytes32 merkleRoot_, uint256 amount_) external;
 
     /**
      * @notice Claim and transfer tokens
@@ -58,7 +59,7 @@ interface IDistribution {
      * Emits a {Claim} event on success.
      */
     function claim(
-        uint256 treeId_,
+        string calldata treeId_,
         string calldata cid_,
         address account_,
         uint256 share_,
@@ -73,5 +74,5 @@ interface IDistribution {
      * @param treeId_ Tree ID
      * @param target_ Address that should receive the unclaimed funds
      */
-    function sweep(uint256 treeId_, address target_) external;
+    function sweep(string calldata treeId_, address target_) external;
 }

src/test/Billboard/DistributionTest.t.sol

@@ -44,31 +44,44 @@ contract DistributionTest is DistributionTestBase {
     function testDrop() public {
         // drop#1
         uint256 _totalAmount = 1510000000000000000;
-        drop(_totalAmount);
-        assertEq(distribution.lastTreeId(), 1);
-        assertEq(distribution.merkleRoots(1), TREE_1_ROOT);
-        assertEq(distribution.balances(1), _totalAmount);
+        string memory _treeId = "1";
+        drop(_treeId, _totalAmount);
+        assertEq(distribution.merkleRoots(_treeId), TREE_1_ROOT);
+        assertEq(distribution.balances(_treeId), _totalAmount);
         assertEq(usdt.balanceOf(address(distribution)), _totalAmount);
 
         // drop#2
-        drop(_totalAmount);
-        assertEq(distribution.lastTreeId(), 2);
-        assertEq(distribution.merkleRoots(2), TREE_1_ROOT);
-        assertEq(distribution.balances(2), _totalAmount);
+        string memory _treeId2 = "2";
+        drop(_treeId2, _totalAmount);
+        assertEq(distribution.merkleRoots(_treeId2), TREE_1_ROOT);
+        assertEq(distribution.balances(_treeId2), _totalAmount);
         assertEq(usdt.balanceOf(address(distribution)), _totalAmount * 2);
     }
 
+    function testCannotDropTwiceWithSameTreeId() public {
+        // drop
+        uint256 _totalAmount = 1510000000000000000;
+        string memory _treeId = "1";
+        drop(_treeId, _totalAmount);
+
+        // drop again
+        deal(address(usdt), ADMIN, _totalAmount);
+        vm.prank(ADMIN);
+        vm.expectRevert("Existing tree");
+        distribution.drop(_treeId, TREE_1_ROOT, _totalAmount);
+    }
+
     function testCannotDropByAttacker() public {
         vm.prank(ATTACKER);
         vm.expectRevert("Admin");
-        distribution.drop(TREE_1_ROOT, 1);
+        distribution.drop("1", TREE_1_ROOT, 1);
     }
 
     function testCannotDropIfZeroAmount() public {
         deal(address(usdt), ADMIN, 0);
         vm.prank(ADMIN);
         vm.expectRevert("Zero amount");
-        distribution.drop(TREE_1_ROOT, 0);
+        distribution.drop("1", TREE_1_ROOT, 0);
     }
 
     function testCannotDropIfInsufficientAllowance(uint256 amount_) public {
@@ -79,7 +92,7 @@ contract DistributionTest is DistributionTestBase {
         usdt.approve(address(distribution), amount_ - 1);
 
         vm.expectRevert("ERC20: insufficient allowance");
-        distribution.drop(TREE_1_ROOT, amount_);
+        distribution.drop("1", TREE_1_ROOT, amount_);
     }
 
     function testCannotDropIfInsufficientBalance(uint256 amount_) public {
@@ -91,24 +104,25 @@ contract DistributionTest is DistributionTestBase {
         usdt.approve(address(distribution), amount_ + 1);
 
         vm.expectRevert("ERC20: transfer amount exceeds balance");
-        distribution.drop(TREE_1_ROOT, amount_ + 1);
+        distribution.drop("1", TREE_1_ROOT, amount_ + 1);
     }
 
     //////////////////////////////
     /// Claim
     //////////////////////////////
     function testClaim() public {
         // drop#1
+        string memory _treeId = "123456-7890";
         uint256 _totalAmount = 1510000000000000000;
-        drop(_totalAmount);
+        drop(_treeId, _totalAmount);
 
         // claim#Alice
         uint256 _amount = distribution.calculateAmount(TREE_1_SHARES[USER_ALICE], _totalAmount);
         vm.expectEmit(true, true, false, false);
         emit IDistribution.Claim(TREE_1_CIDS[USER_ALICE], USER_ALICE, _amount);
         uint256 balanceAlce = address(USER_ALICE).balance;
         distribution.claim(
-            1,
+            _treeId,
             TREE_1_CIDS[USER_ALICE],
             USER_ALICE,
             TREE_1_SHARES[USER_ALICE],
@@ -122,7 +136,7 @@ contract DistributionTest is DistributionTestBase {
         vm.expectEmit(true, true, false, false);
         emit IDistribution.Claim(TREE_1_CIDS[USER_BOB], USER_BOB, _amountBob);
         uint256 balanceBob = address(USER_BOB).balance;
-        distribution.claim(1, TREE_1_CIDS[USER_BOB], USER_BOB, TREE_1_SHARES[USER_BOB], TREE_1_PROOFS[USER_BOB]);
+        distribution.claim(_treeId, TREE_1_CIDS[USER_BOB], USER_BOB, TREE_1_SHARES[USER_BOB], TREE_1_PROOFS[USER_BOB]);
         assertEq(usdt.balanceOf(address(USER_BOB)), balanceBob + _amountBob);
 
         // claim#Charlie
@@ -131,7 +145,7 @@ contract DistributionTest is DistributionTestBase {
         emit IDistribution.Claim(TREE_1_CIDS[USER_CHARLIE], USER_CHARLIE, _amountCharlie);
         uint256 balanceCharlie = address(USER_CHARLIE).balance;
         distribution.claim(
-            1,
+            _treeId,
             TREE_1_CIDS[USER_CHARLIE],
             USER_CHARLIE,
             TREE_1_SHARES[USER_CHARLIE],
@@ -145,12 +159,13 @@ contract DistributionTest is DistributionTestBase {
 
     function testCannotClaimIfAlreadyClaimed() public {
         // drop#1
+        string memory _treeId = "1";
         uint256 _totalAmount = 1510000000000000000;
-        drop(_totalAmount);
+        drop(_treeId, _totalAmount);
 
         // claim#Alice
         distribution.claim(
-            1,
+            _treeId,
             TREE_1_CIDS[USER_ALICE],
             USER_ALICE,
             TREE_1_SHARES[USER_ALICE],
@@ -160,7 +175,7 @@ contract DistributionTest is DistributionTestBase {
         // claim#Alice again
         vm.expectRevert("Already claimed");
         distribution.claim(
-            1,
+            _treeId,
             TREE_1_CIDS[USER_ALICE],
             USER_ALICE,
             TREE_1_SHARES[USER_ALICE],
@@ -170,24 +185,27 @@ contract DistributionTest is DistributionTestBase {
 
     function testCannotClaimIfInvalidProof() public {
         // drop#1
+        string memory _treeId = "1";
         uint256 _totalAmount = 1510000000000000000;
-        drop(_totalAmount);
+        drop(_treeId, _totalAmount);
 
         // claim#Alice
         uint256 _amount = distribution.calculateAmount(TREE_1_SHARES[USER_ALICE], _totalAmount);
         vm.expectRevert("Invalid proof");
-        distribution.claim(1, TREE_1_CIDS[USER_ALICE], USER_ALICE, _amount, TREE_1_PROOFS[USER_BOB]);
+        distribution.claim(_treeId, TREE_1_CIDS[USER_ALICE], USER_ALICE, _amount, TREE_1_PROOFS[USER_BOB]);
     }
 
     function testCannotClaimIfInvalidTreeId() public {
         // drop#1
+        string memory _treeId = "1";
         uint256 _totalAmount = 1510000000000000000;
-        drop(_totalAmount);
+        drop(_treeId, _totalAmount);
 
         // claim#Alice
+        string memory _treeId2 = "2";
         vm.expectRevert("Invalid tree ID");
         distribution.claim(
-            2,
+            _treeId2,
             TREE_1_CIDS[USER_ALICE],
             USER_ALICE,
             TREE_1_SHARES[USER_ALICE],
@@ -197,15 +215,16 @@ contract DistributionTest is DistributionTestBase {
 
     function testCannotClaimIfInsufficientBalance() public {
         // drop#1
+        string memory _treeId = "1";
         uint256 _totalAmount = 1510000000000000000;
-        drop(_totalAmount);
+        drop(_treeId, _totalAmount);
         deal(address(usdt), address(distribution), 0);
         assertEq(usdt.balanceOf(address(distribution)), 0);
 
         // claim#Alice
         vm.expectRevert("ERC20: transfer amount exceeds balance");
         distribution.claim(
-            1,
+            _treeId,
             TREE_1_CIDS[USER_ALICE],
             USER_ALICE,
             TREE_1_SHARES[USER_ALICE],
@@ -218,37 +237,41 @@ contract DistributionTest is DistributionTestBase {
     //////////////////////////////
     function testSweep() public {
         // drop
+        string memory _treeId = "1";
         uint256 _totalAmount = 1510000000000000000;
-        drop(_totalAmount);
+        drop(_treeId, _totalAmount);
 
         // sweep
         uint256 prevBalance = usdt.balanceOf(ADMIN);
         vm.prank(ADMIN);
-        distribution.sweep(1, ADMIN);
+        distribution.sweep(_treeId, ADMIN);
         assertEq(usdt.balanceOf(ADMIN), prevBalance + _totalAmount);
         assertEq(usdt.balanceOf(address(distribution)), 0);
-        assertEq(distribution.balances(1), 0);
+        assertEq(distribution.balances(_treeId), 0);
     }
 
     function testCannotSweepByAttacker() public {
         // drop
+        string memory _treeId = "1";
         uint256 _totalAmount = 1510000000000000000;
-        drop(_totalAmount);
+        drop(_treeId, _totalAmount);
 
         // sweep
         vm.prank(ATTACKER);
         vm.expectRevert("Admin");
-        distribution.sweep(1, ADMIN);
+        distribution.sweep(_treeId, ADMIN);
     }
 
     function testCannotSweepIfZeroBalance() public {
         // drop
+        string memory _treeId = "1";
         uint256 _totalAmount = 1510000000000000000;
-        drop(_totalAmount);
+        drop(_treeId, _totalAmount);
 
         // sweep
+        string memory _sweepTreeId = "2";
         vm.prank(ADMIN);
         vm.expectRevert("Zero balance");
-        distribution.sweep(2, ADMIN);
+        distribution.sweep(_sweepTreeId, ADMIN);
     }
 }

src/test/Billboard/DistributionTestBase.t.sol

@@ -84,9 +84,9 @@ contract DistributionTestBase is Test {
         usdt.approve(address(distribution), MAX_ALLOWANCE);
     }
 
-    function drop(uint256 amount_) public {
+    function drop(string memory treeId_, uint256 amount_) public {
         deal(address(usdt), ADMIN, amount_);
         vm.prank(ADMIN);
-        distribution.drop(TREE_1_ROOT, amount_);
+        distribution.drop(treeId_, TREE_1_ROOT, amount_);
     }
 }