From c20ea916ee5655c9d4b71219908a993db207f7c4 Mon Sep 17 00:00:00 2001 From: "Julian M. Rodriguez" <56316686+julianmrodri@users.noreply.github.com> Date: Tue, 24 Oct 2023 13:19:52 -0300 Subject: [PATCH] TRUST QA-6/7: Optimizations in Distributor (#988) Co-authored-by: Taylor Brent --- contracts/facade/FacadeTest.sol | 3 +++ contracts/p0/Distributor.sol | 4 ++-- contracts/p1/Distributor.sol | 13 ++++--------- contracts/plugins/mocks/RevenueTraderBackComp.sol | 4 +++- test/Revenues.test.ts | 11 +++++++++++ 5 files changed, 23 insertions(+), 12 deletions(-) diff --git a/contracts/facade/FacadeTest.sol b/contracts/facade/FacadeTest.sol index 78ee2173e0..9b57538a4c 100644 --- a/contracts/facade/FacadeTest.sol +++ b/contracts/facade/FacadeTest.sol @@ -66,6 +66,7 @@ contract FacadeTest is IFacadeTest { erc20s ); try main.rsrTrader().manageTokens(rsrERC20s, rsrKinds) {} catch {} + try main.rsrTrader().distributeTokenToBuy() {} catch {} // Start exact RToken auctions (IERC20[] memory rTokenERC20s, TradeKind[] memory rTokenKinds) = traderERC20s( @@ -74,6 +75,7 @@ contract FacadeTest is IFacadeTest { erc20s ); try main.rTokenTrader().manageTokens(rTokenERC20s, rTokenKinds) {} catch {} + try main.rTokenTrader().distributeTokenToBuy() {} catch {} // solhint-enable no-empty-blocks } @@ -133,6 +135,7 @@ contract FacadeTest is IFacadeTest { IERC20[] memory traderERC20sAll = new IERC20[](erc20sAll.length); for (uint256 i = 0; i < erc20sAll.length; ++i) { if ( + erc20sAll[i] != trader.tokenToBuy() && address(trader.trades(erc20sAll[i])) == address(0) && erc20sAll[i].balanceOf(address(trader)) > 1 ) { diff --git a/contracts/p0/Distributor.sol b/contracts/p0/Distributor.sol index 9f43240c5e..264d7bfe7e 100644 --- a/contracts/p0/Distributor.sol +++ b/contracts/p0/Distributor.sol @@ -63,8 +63,8 @@ contract DistributorP0 is ComponentP0, IDistributor { { RevenueTotals memory revTotals = totals(); uint256 totalShares = isRSR ? revTotals.rsrTotal : revTotals.rTokenTotal; - require(totalShares > 0, "nothing to distribute"); - tokensPerShare = amount / totalShares; + if (totalShares > 0) tokensPerShare = amount / totalShares; + require(tokensPerShare > 0, "nothing to distribute"); } // Evenly distribute revenue tokens per distribution share. diff --git a/contracts/p1/Distributor.sol b/contracts/p1/Distributor.sol index 04fedb57ee..776e19fe5a 100644 --- a/contracts/p1/Distributor.sol +++ b/contracts/p1/Distributor.sol @@ -68,7 +68,6 @@ contract DistributorP1 is ComponentP1, IDistributor { } struct Transfer { - IERC20 erc20; address addrTo; uint256 amount; } @@ -99,8 +98,8 @@ contract DistributorP1 is ComponentP1, IDistributor { { RevenueTotals memory revTotals = totals(); uint256 totalShares = isRSR ? revTotals.rsrTotal : revTotals.rTokenTotal; - require(totalShares > 0, "nothing to distribute"); - tokensPerShare = amount / totalShares; + if (totalShares > 0) tokensPerShare = amount / totalShares; + require(tokensPerShare > 0, "nothing to distribute"); } // Evenly distribute revenue tokens per distribution share. @@ -131,11 +130,7 @@ contract DistributorP1 is ComponentP1, IDistributor { if (transferAmt > 0) accountRewards = true; } - transfers[numTransfers] = Transfer({ - erc20: erc20, - addrTo: addrTo, - amount: transferAmt - }); + transfers[numTransfers] = Transfer({ addrTo: addrTo, amount: transferAmt }); numTransfers++; } emit RevenueDistributed(erc20, caller, amount); @@ -143,7 +138,7 @@ contract DistributorP1 is ComponentP1, IDistributor { // == Interactions == for (uint256 i = 0; i < numTransfers; i++) { Transfer memory t = transfers[i]; - IERC20Upgradeable(address(t.erc20)).safeTransferFrom(caller, t.addrTo, t.amount); + IERC20Upgradeable(address(erc20)).safeTransferFrom(caller, t.addrTo, t.amount); } // Perform reward accounting diff --git a/contracts/plugins/mocks/RevenueTraderBackComp.sol b/contracts/plugins/mocks/RevenueTraderBackComp.sol index ed76f53346..73069f15ad 100644 --- a/contracts/plugins/mocks/RevenueTraderBackComp.sol +++ b/contracts/plugins/mocks/RevenueTraderBackComp.sol @@ -14,8 +14,10 @@ contract RevenueTraderCompatibleV2 is RevenueTraderP1, IRevenueTraderComp { erc20s[0] = sell; TradeKind[] memory kinds = new TradeKind[](1); kinds[0] = TradeKind.DUTCH_AUCTION; + // Mirror V3 logic (only the section relevant to tests) - this.manageTokens(erc20s, kinds); + // solhint-disable-next-line no-empty-blocks + try this.manageTokens(erc20s, kinds) {} catch {} } function version() public pure virtual override(Versioned, IVersioned) returns (string memory) { diff --git a/test/Revenues.test.ts b/test/Revenues.test.ts index 1c7ad79df4..3858ba4290 100644 --- a/test/Revenues.test.ts +++ b/test/Revenues.test.ts @@ -706,6 +706,17 @@ describe(`Revenues - P${IMPLEMENTATION}`, () => { expect(newRTokenTotal).equal(bn(0)) }) + it('Should avoid zero transfers when distributing tokenToBuy', async () => { + // Distribute with no balance + await expect(rsrTrader.distributeTokenToBuy()).to.be.revertedWith('nothing to distribute') + expect(await rsr.balanceOf(stRSR.address)).to.equal(bn(0)) + + // Small amount which ends in zero distribution due to rounding + await rsr.connect(owner).mint(rsrTrader.address, bn(1)) + await expect(rsrTrader.distributeTokenToBuy()).to.be.revertedWith('nothing to distribute') + expect(await rsr.balanceOf(stRSR.address)).to.equal(bn(0)) + }) + it('Should account rewards when distributing tokenToBuy', async () => { // 1. StRSR.payoutRewards() const stRSRBal = await rsr.balanceOf(stRSR.address)