From e411dcb76a09bdc8416860a62cafdb5226eb9d4d Mon Sep 17 00:00:00 2001 From: patricio henderson Date: Mon, 15 Apr 2024 09:33:27 -0300 Subject: [PATCH] Tasks: Apply suggestions from code review --- .../interfaces/swap/IBalancerV2Swapper.sol | 9 ++------- packages/tasks/contracts/swap/BalancerV2Swapper.sol | 12 +----------- packages/tasks/test/swap/BalancerV2Swapper.test.ts | 13 ++++--------- 3 files changed, 7 insertions(+), 27 deletions(-) diff --git a/packages/tasks/contracts/interfaces/swap/IBalancerV2Swapper.sol b/packages/tasks/contracts/interfaces/swap/IBalancerV2Swapper.sol index 1f66cd0..4c44862 100644 --- a/packages/tasks/contracts/interfaces/swap/IBalancerV2Swapper.sol +++ b/packages/tasks/contracts/interfaces/swap/IBalancerV2Swapper.sol @@ -26,20 +26,15 @@ interface IBalancerV2Swapper is IBaseSwapTask { error TaskMissingPoolId(); /** - * @dev The pool id for the token zero + * @dev The requested pool id to be set is zero */ - error PoolIdZero(); + error TaskPoolIdZero(); /** * @dev Emitted every time a pool is set for a token */ event BalancerPoolIdSet(address indexed token, bytes32 poolId); - /** - * @dev Tells pool id set for a token - */ - function tokenPoolId(address token) external view returns (bytes32); - /** * @dev Execution function */ diff --git a/packages/tasks/contracts/swap/BalancerV2Swapper.sol b/packages/tasks/contracts/swap/BalancerV2Swapper.sol index 1f8f676..fb62451 100644 --- a/packages/tasks/contracts/swap/BalancerV2Swapper.sol +++ b/packages/tasks/contracts/swap/BalancerV2Swapper.sol @@ -86,16 +86,6 @@ contract BalancerV2Swapper is IBalancerV2Swapper, BaseSwapTask { _setPoolId(token, poolId); } - /** - * @dev Tells pool id set for a token - * @param token address of the token - */ - function tokenPoolId(address token) external view returns (bytes32) { - bytes32 poolId = balancerPoolId[token]; - require(poolId != bytes32(0), 'Pool id not found for token'); - return poolId; - } - /** * @dev Executes the Balancer v2 swapper task */ @@ -155,7 +145,7 @@ contract BalancerV2Swapper is IBalancerV2Swapper, BaseSwapTask { */ function _setPoolId(address token, bytes32 poolId) internal { if (token == address(0)) revert TaskTokenZero(); - if (poolId == bytes32(0)) revert PoolIdZero(); + if (poolId == bytes32(0)) revert TaskPoolIdZero(); balancerPoolId[token] = poolId; emit BalancerPoolIdSet(token, poolId); diff --git a/packages/tasks/test/swap/BalancerV2Swapper.test.ts b/packages/tasks/test/swap/BalancerV2Swapper.test.ts index 043ebcd..6563f5a 100644 --- a/packages/tasks/test/swap/BalancerV2Swapper.test.ts +++ b/packages/tasks/test/swap/BalancerV2Swapper.test.ts @@ -79,11 +79,6 @@ describe('BalancerV2Swapper', () => { context('when the pool id is not zero', () => { const poolId = ONES_BYTES32 - it('sets the pool id', async () => { - await task.setPoolId(token.address, poolId) - expect(await task.tokenPoolId(token.address)).to.be.equal(poolId) - }) - it('emits an event', async () => { const tx = await task.setPoolId(token.address, poolId) await assertEvent(tx, 'BalancerPoolIdSet', { token: token.address, poolId }) @@ -95,9 +90,9 @@ describe('BalancerV2Swapper', () => { }) it('updates the pool id', async () => { - const newPoolId = '0x0000000000000000000000000000000000000000000000000000000000000001' - await task.setPoolId(token.address, newPoolId) - expect(await task.tokenPoolId(token.address)).to.be.equal(newPoolId) + const poolId = '0x0000000000000000000000000000000000000000000000000000000000000001' + const tx = await task.setPoolId(token.address, poolId) + await assertEvent(tx, 'BalancerPoolIdSet', { token: token.address, poolId }) }) }) }) @@ -106,7 +101,7 @@ describe('BalancerV2Swapper', () => { const poolId = ZERO_BYTES32 it('reverts', async () => { - await expect(task.setPoolId(token.address, poolId)).to.be.revertedWith('PoolIdZero') + await expect(task.setPoolId(token.address, poolId)).to.be.revertedWith('TaskPoolIdZero') }) })