From c416e7c597b36b0eaeba42ed55b71fbac9a71fb9 Mon Sep 17 00:00:00 2001 From: FelixFan1992 Date: Fri, 26 Jul 2024 09:43:55 -0400 Subject: [PATCH 01/13] auto: migrate more tests to foundry --- .../test/v2_3/AutomationRegistry2_3.t.sol | 33 +++++++++ .../automation/AutomationRegistry2_3.test.ts | 68 +++++++++---------- 2 files changed, 67 insertions(+), 34 deletions(-) diff --git a/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol b/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol index dbc0c203c07..fe0db2948f0 100644 --- a/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol +++ b/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol @@ -1977,3 +1977,36 @@ contract MigrateReceive is SetUp { vm.stopPrank(); } } + +contract Pause is SetUp { + function test_RevertsWhen_CalledByNonOwner() external { + vm.expectRevert(bytes("Only callable by owner")); + vm.prank(STRANGER); + registry.pause(); + } + + function test_CalledByOwner_success() external { + vm.startPrank(registry.owner()); + registry.pause(); + + (IAutomationV21PlusCommon.StateLegacy memory state, , , , ) = registry.getState(); + assertTrue(state.paused); + } + + function test_revertsWhen_registerUpkeepInPausedRegistry() external { + vm.startPrank(registry.owner()); + registry.pause(); + + vm.expectRevert(Registry.RegistryPaused.selector); + linkUpkeepID = registry.registerUpkeep( + address(TARGET1), + config.maxPerformGas, + UPKEEP_ADMIN, + uint8(Trigger.CONDITION), + address(linkToken), + "", + "", + "" + ); + } +} diff --git a/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts b/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts index 9a572269695..53c1ffefc7e 100644 --- a/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts +++ b/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts @@ -4890,20 +4890,20 @@ describe('AutomationRegistry2_3', () => { }) describe('#pause', () => { - it('reverts if called by a non-owner', async () => { - await evmRevert( - registry.connect(keeper1).pause(), - 'Only callable by owner', - ) - }) - - it('marks the contract as paused', async () => { - assert.isFalse((await registry.getState()).state.paused) - - await registry.connect(owner).pause() - - assert.isTrue((await registry.getState()).state.paused) - }) + // it('reverts if called by a non-owner', async () => { + // await evmRevert( + // registry.connect(keeper1).pause(), + // 'Only callable by owner', + // ) + // }) + // + // it('marks the contract as paused', async () => { + // assert.isFalse((await registry.getState()).state.paused) + // + // await registry.connect(owner).pause() + // + // assert.isTrue((await registry.getState()).state.paused) + // }) it('Does not allow transmits when paused', async () => { await registry.connect(owner).pause() @@ -4915,26 +4915,26 @@ describe('AutomationRegistry2_3', () => { ) }) - it('Does not allow creation of new upkeeps when paused', async () => { - await registry.connect(owner).pause() - - await evmRevertCustomError( - registry - .connect(owner) - .registerUpkeep( - mock.address, - performGas, - await admin.getAddress(), - Trigger.CONDITION, - linkToken.address, - '0x', - '0x', - '0x', - ), - registry, - 'RegistryPaused', - ) - }) + // it('Does not allow creation of new upkeeps when paused', async () => { + // await registry.connect(owner).pause() + // + // await evmRevertCustomError( + // registry + // .connect(owner) + // .registerUpkeep( + // mock.address, + // performGas, + // await admin.getAddress(), + // Trigger.CONDITION, + // linkToken.address, + // '0x', + // '0x', + // '0x', + // ), + // registry, + // 'RegistryPaused', + // ) + // }) }) describe('#unpause', () => { From ab74f94bc4ddd2a8bbf0d52472d8534e7e67104e Mon Sep 17 00:00:00 2001 From: FelixFan1992 Date: Fri, 26 Jul 2024 11:05:20 -0400 Subject: [PATCH 02/13] more tests --- .../test/v2_3/AutomationRegistry2_3.t.sol | 130 +++++++++++++++- .../automation/AutomationRegistry2_3.test.ts | 145 ------------------ 2 files changed, 129 insertions(+), 146 deletions(-) diff --git a/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol b/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol index fe0db2948f0..f36cbb3a175 100644 --- a/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol +++ b/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol @@ -1998,7 +1998,7 @@ contract Pause is SetUp { registry.pause(); vm.expectRevert(Registry.RegistryPaused.selector); - linkUpkeepID = registry.registerUpkeep( + uint256 id = registry.registerUpkeep( address(TARGET1), config.maxPerformGas, UPKEEP_ADMIN, @@ -2009,4 +2009,132 @@ contract Pause is SetUp { "" ); } + + // function test_revertsWhen_transmitInPausedRegistry() external { + // vm.startPrank(registry.owner()); + // registry.pause(); + // + // vm.expectRevert(Registry.RegistryPaused.selector); + // _transmit(usdUpkeepID18, registry); + // } +} + +contract Unpause is SetUp { + function test_RevertsWhen_CalledByNonOwner() external { + vm.startPrank(registry.owner()); + registry.pause(); + + vm.expectRevert(bytes("Only callable by owner")); + vm.startPrank(STRANGER); + registry.unpause(); + } + + function test_CalledByOwner_success() external { + vm.startPrank(registry.owner()); + registry.pause(); + (IAutomationV21PlusCommon.StateLegacy memory state1, , , , ) = registry.getState(); + assertTrue(state1.paused); + + registry.unpause(); + (IAutomationV21PlusCommon.StateLegacy memory state2, , , , ) = registry.getState(); + assertFalse(state2.paused); + } +} + +contract CancelUpkeep is SetUp { + event UpkeepCanceled(uint256 indexed id, uint64 indexed atBlockHeight); + + function test_RevertsWhen_IdIsInvalid_CalledByAdmin() external { + vm.startPrank(UPKEEP_ADMIN); + vm.expectRevert(Registry.CannotCancel.selector); + registry.cancelUpkeep(1111111); + } + + function test_RevertsWhen_IdIsInvalid_CalledByOwner() external { + vm.startPrank(registry.owner()); + vm.expectRevert(Registry.CannotCancel.selector); + registry.cancelUpkeep(1111111); + } + + function test_RevertsWhen_NotCalledByOwnerOrAdmin() external { + vm.startPrank(STRANGER); + vm.expectRevert(Registry.OnlyCallableByOwnerOrAdmin.selector); + registry.cancelUpkeep(linkUpkeepID); + } + + function test_RevertsWhen_UpkeepAlreadyCanceledByAdmin_CalledByOwner() external { + uint256 bn = block.number; + vm.startPrank(UPKEEP_ADMIN); + registry.cancelUpkeep(linkUpkeepID); + + vm.roll(50 + bn); + + vm.startPrank(registry.owner()); + vm.expectRevert(Registry.UpkeepCancelled.selector); + registry.cancelUpkeep(linkUpkeepID); + } + + function test_RevertsWhen_UpkeepAlreadyCanceled_CalledByAdmin() external { + uint256 bn = block.number; + vm.startPrank(UPKEEP_ADMIN); + registry.cancelUpkeep(linkUpkeepID); + + vm.roll(10 + bn); + + vm.expectRevert(Registry.UpkeepCancelled.selector); + registry.cancelUpkeep(linkUpkeepID); + } + + function test_RevertsWhen_UpkeepAlreadyCanceled_CalledByOwner() external { + uint256 bn = block.number; + vm.startPrank(registry.owner()); + registry.cancelUpkeep(linkUpkeepID); + + vm.roll(10 + bn); + + vm.expectRevert(Registry.UpkeepCancelled.selector); + registry.cancelUpkeep(linkUpkeepID); + } + + function test_CancelUpkeep_SetMaxValidBlockNumber_CalledByAdmin() external { + uint256 bn = block.number; + vm.startPrank(UPKEEP_ADMIN); + registry.cancelUpkeep(linkUpkeepID); + + vm.roll(10 + bn); + uint256 maxValidBlocknumber = uint256(registry.getUpkeep(linkUpkeepID).maxValidBlocknumber); + + // 50 is the cancellation delay + assertEq(bn + 50, maxValidBlocknumber); + } + + function test_CancelUpkeep_SetMaxValidBlockNumber_CalledByOwner() external { + uint256 bn = block.number; + vm.startPrank(registry.owner()); + registry.cancelUpkeep(linkUpkeepID); + + vm.roll(10 + bn); + uint256 maxValidBlocknumber = uint256(registry.getUpkeep(linkUpkeepID).maxValidBlocknumber); + + // cancellation by registry owner is immediate and no cancellation delay is applied + assertEq(bn, maxValidBlocknumber); + } + + function test_cancelUpkeep_emitEvent_CalledByAdmin() external { + uint256 bn = block.number; + vm.startPrank(UPKEEP_ADMIN); + + vm.expectEmit(); + emit UpkeepCanceled(linkUpkeepID, uint64(bn + 50)); + registry.cancelUpkeep(linkUpkeepID); + } + + function test_cancelUpkeep_emitEvent_CalledByOwner() external { + uint256 bn = block.number; + vm.startPrank(registry.owner()); + + vm.expectEmit(); + emit UpkeepCanceled(linkUpkeepID, uint64(bn)); + registry.cancelUpkeep(linkUpkeepID); + } } diff --git a/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts b/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts index 53c1ffefc7e..52f8e23226f 100644 --- a/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts +++ b/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts @@ -4890,21 +4890,6 @@ describe('AutomationRegistry2_3', () => { }) describe('#pause', () => { - // it('reverts if called by a non-owner', async () => { - // await evmRevert( - // registry.connect(keeper1).pause(), - // 'Only callable by owner', - // ) - // }) - // - // it('marks the contract as paused', async () => { - // assert.isFalse((await registry.getState()).state.paused) - // - // await registry.connect(owner).pause() - // - // assert.isTrue((await registry.getState()).state.paused) - // }) - it('Does not allow transmits when paused', async () => { await registry.connect(owner).pause() @@ -4914,48 +4899,6 @@ describe('AutomationRegistry2_3', () => { 'RegistryPaused', ) }) - - // it('Does not allow creation of new upkeeps when paused', async () => { - // await registry.connect(owner).pause() - // - // await evmRevertCustomError( - // registry - // .connect(owner) - // .registerUpkeep( - // mock.address, - // performGas, - // await admin.getAddress(), - // Trigger.CONDITION, - // linkToken.address, - // '0x', - // '0x', - // '0x', - // ), - // registry, - // 'RegistryPaused', - // ) - // }) - }) - - describe('#unpause', () => { - beforeEach(async () => { - await registry.connect(owner).pause() - }) - - it('reverts if called by a non-owner', async () => { - await evmRevert( - registry.connect(keeper1).unpause(), - 'Only callable by owner', - ) - }) - - it('marks the contract as not paused', async () => { - assert.isTrue((await registry.getState()).state.paused) - - await registry.connect(owner).unpause() - - assert.isFalse((await registry.getState()).state.paused) - }) }) describe('#setPayees', () => { @@ -5078,41 +5021,7 @@ describe('AutomationRegistry2_3', () => { }) describe('#cancelUpkeep', () => { - it('reverts if the ID is not valid', async () => { - await evmRevertCustomError( - registry.connect(owner).cancelUpkeep(upkeepId.add(1)), - registry, - 'CannotCancel', - ) - }) - - it('reverts if called by a non-owner/non-admin', async () => { - await evmRevertCustomError( - registry.connect(keeper1).cancelUpkeep(upkeepId), - registry, - 'OnlyCallableByOwnerOrAdmin', - ) - }) - describe('when called by the owner', async () => { - it('sets the registration to invalid immediately', async () => { - const tx = await registry.connect(owner).cancelUpkeep(upkeepId) - const receipt = await tx.wait() - const registration = await registry.getUpkeep(upkeepId) - assert.equal( - registration.maxValidBlocknumber.toNumber(), - receipt.blockNumber, - ) - }) - - it('emits an event', async () => { - const tx = await registry.connect(owner).cancelUpkeep(upkeepId) - const receipt = await tx.wait() - await expect(tx) - .to.emit(registry, 'UpkeepCanceled') - .withArgs(upkeepId, BigNumber.from(receipt.blockNumber)) - }) - it('immediately prevents upkeep', async () => { await registry.connect(owner).cancelUpkeep(upkeepId) @@ -5124,15 +5033,6 @@ describe('AutomationRegistry2_3', () => { assert.equal(cancelledUpkeepReportLogs.length, 1) }) - it('does not revert if reverts if called multiple times', async () => { - await registry.connect(owner).cancelUpkeep(upkeepId) - await evmRevertCustomError( - registry.connect(owner).cancelUpkeep(upkeepId), - registry, - 'UpkeepCancelled', - ) - }) - describe('when called by the owner when the admin has just canceled', () => { // eslint-disable-next-line @typescript-eslint/no-unused-vars // @ts-ignore @@ -5155,51 +5055,6 @@ describe('AutomationRegistry2_3', () => { }) describe('when called by the admin', async () => { - it('reverts if called again by the admin', async () => { - await registry.connect(admin).cancelUpkeep(upkeepId) - - await evmRevertCustomError( - registry.connect(admin).cancelUpkeep(upkeepId), - registry, - 'UpkeepCancelled', - ) - }) - - it('reverts if called by the owner after the timeout', async () => { - await registry.connect(admin).cancelUpkeep(upkeepId) - - for (let i = 0; i < cancellationDelay; i++) { - await ethers.provider.send('evm_mine', []) - } - - await evmRevertCustomError( - registry.connect(owner).cancelUpkeep(upkeepId), - registry, - 'UpkeepCancelled', - ) - }) - - it('sets the registration to invalid in 50 blocks', async () => { - const tx = await registry.connect(admin).cancelUpkeep(upkeepId) - const receipt = await tx.wait() - const registration = await registry.getUpkeep(upkeepId) - assert.equal( - registration.maxValidBlocknumber.toNumber(), - receipt.blockNumber + 50, - ) - }) - - it('emits an event', async () => { - const tx = await registry.connect(admin).cancelUpkeep(upkeepId) - const receipt = await tx.wait() - await expect(tx) - .to.emit(registry, 'UpkeepCanceled') - .withArgs( - upkeepId, - BigNumber.from(receipt.blockNumber + cancellationDelay), - ) - }) - it('immediately prevents upkeep', async () => { await linkToken.connect(owner).approve(registry.address, toWei('100')) await registry.connect(owner).addFunds(upkeepId, toWei('100')) From de20272b2e39626d06df32cd6900094a22dccb9a Mon Sep 17 00:00:00 2001 From: FelixFan1992 Date: Fri, 26 Jul 2024 12:34:57 -0400 Subject: [PATCH 03/13] tests for migration permission --- .../test/v2_3/AutomationRegistry2_3.t.sol | 42 ++++++++++++++++++- .../automation/AutomationRegistry2_3.test.ts | 27 ------------ 2 files changed, 40 insertions(+), 29 deletions(-) diff --git a/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol b/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol index f36cbb3a175..50d415b8ffa 100644 --- a/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol +++ b/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol @@ -2120,7 +2120,7 @@ contract CancelUpkeep is SetUp { assertEq(bn, maxValidBlocknumber); } - function test_cancelUpkeep_emitEvent_CalledByAdmin() external { + function test_CancelUpkeep_EmitEvent_CalledByAdmin() external { uint256 bn = block.number; vm.startPrank(UPKEEP_ADMIN); @@ -2129,7 +2129,7 @@ contract CancelUpkeep is SetUp { registry.cancelUpkeep(linkUpkeepID); } - function test_cancelUpkeep_emitEvent_CalledByOwner() external { + function test_CancelUpkeep_EmitEvent_CalledByOwner() external { uint256 bn = block.number; vm.startPrank(registry.owner()); @@ -2138,3 +2138,41 @@ contract CancelUpkeep is SetUp { registry.cancelUpkeep(linkUpkeepID); } } + +contract SetPeerRegistryMigrationPermission is SetUp { + function test_SetPeerRegistryMigrationPermission_CalledByOwner() external { + address peer = randomAddress(); + vm.startPrank(registry.owner()); + + uint8 permission = registry.getPeerRegistryMigrationPermission(peer); + assertEq(0, permission); + + registry.setPeerRegistryMigrationPermission(peer, 1); + permission = registry.getPeerRegistryMigrationPermission(peer); + assertEq(1, permission); + + registry.setPeerRegistryMigrationPermission(peer, 2); + permission = registry.getPeerRegistryMigrationPermission(peer); + assertEq(2, permission); + + registry.setPeerRegistryMigrationPermission(peer, 0); + permission = registry.getPeerRegistryMigrationPermission(peer); + assertEq(0, permission); + } + + function test_RevertsWhen_InvalidPermission_CalledByOwner() external { + address peer = randomAddress(); + vm.startPrank(registry.owner()); + + vm.expectRevert(); + registry.setPeerRegistryMigrationPermission(peer, 100); + } + + function test_RevertsWhen_CalledByNonOwner() external { + address peer = randomAddress(); + vm.startPrank(STRANGER); + + vm.expectRevert(bytes("Only callable by owner")); + registry.setPeerRegistryMigrationPermission(peer, 1); + } +} diff --git a/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts b/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts index 52f8e23226f..4757798ebc2 100644 --- a/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts +++ b/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts @@ -4225,33 +4225,6 @@ describe('AutomationRegistry2_3', () => { }) }) - describe('#setPeerRegistryMigrationPermission() / #getPeerRegistryMigrationPermission()', () => { - const peer = randomAddress() - it('allows the owner to set the peer registries', async () => { - let permission = await registry.getPeerRegistryMigrationPermission(peer) - expect(permission).to.equal(0) - await registry.setPeerRegistryMigrationPermission(peer, 1) - permission = await registry.getPeerRegistryMigrationPermission(peer) - expect(permission).to.equal(1) - await registry.setPeerRegistryMigrationPermission(peer, 2) - permission = await registry.getPeerRegistryMigrationPermission(peer) - expect(permission).to.equal(2) - await registry.setPeerRegistryMigrationPermission(peer, 0) - permission = await registry.getPeerRegistryMigrationPermission(peer) - expect(permission).to.equal(0) - }) - it('reverts if passed an unsupported permission', async () => { - await expect( - registry.connect(admin).setPeerRegistryMigrationPermission(peer, 10), - ).to.be.reverted - }) - it('reverts if not called by the owner', async () => { - await expect( - registry.connect(admin).setPeerRegistryMigrationPermission(peer, 1), - ).to.be.revertedWith('Only callable by owner') - }) - }) - describe('#pauseUpkeep', () => { it('reverts if the registration does not exist', async () => { await evmRevertCustomError( From 0dd5990f8d2c16721b1286e324f0fa0d46e7bb62 Mon Sep 17 00:00:00 2001 From: FelixFan1992 Date: Fri, 26 Jul 2024 14:09:35 -0400 Subject: [PATCH 04/13] tests for upkeep and admin configs --- .../test/v2_3/AutomationRegistry2_3.t.sol | 47 +++++++++++++++ .../automation/AutomationRegistry2_3.test.ts | 57 ------------------- 2 files changed, 47 insertions(+), 57 deletions(-) diff --git a/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol b/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol index 50d415b8ffa..460c7d2fdb6 100644 --- a/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol +++ b/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol @@ -2176,3 +2176,50 @@ contract SetPeerRegistryMigrationPermission is SetUp { registry.setPeerRegistryMigrationPermission(peer, 1); } } + +contract SetUpkeepPrivilegeConfig is SetUp { + function test_RevertsWhen_CalledByNonManager() external { + vm.startPrank(STRANGER); + + vm.expectRevert(Registry.OnlyCallableByUpkeepPrivilegeManager.selector); + registry.setUpkeepPrivilegeConfig(linkUpkeepID, hex"1233"); + } + + function test_UpkeepHasEmptyConfig() external { + bytes memory cfg = registry.getUpkeepPrivilegeConfig(linkUpkeepID); + assertEq(cfg, bytes("")); + } + + function test_SetUpkeepPrivilegeConfig_CalledByManager() external { + vm.startPrank(PRIVILEGE_MANAGER); + + registry.setUpkeepPrivilegeConfig(linkUpkeepID, hex"1233"); + + bytes memory cfg = registry.getUpkeepPrivilegeConfig(linkUpkeepID); + assertEq(cfg, hex"1233"); + } +} + +contract SetAdminPrivilegeConfig is SetUp { + function test_RevertsWhen_CalledByNonManager() external { + vm.startPrank(STRANGER); + + vm.expectRevert(Registry.OnlyCallableByUpkeepPrivilegeManager.selector); + registry.setAdminPrivilegeConfig(randomAddress(), hex"1233"); + } + + function test_UpkeepHasEmptyConfig() external { + bytes memory cfg = registry.getAdminPrivilegeConfig(randomAddress()); + assertEq(cfg, bytes("")); + } + + function test_SetAdminPrivilegeConfig_CalledByManager() external { + vm.startPrank(PRIVILEGE_MANAGER); + address admin = randomAddress(); + + registry.setAdminPrivilegeConfig(admin, hex"1233"); + + bytes memory cfg = registry.getAdminPrivilegeConfig(admin); + assertEq(cfg, hex"1233"); + } +} diff --git a/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts b/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts index 4757798ebc2..47c901c8bd2 100644 --- a/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts +++ b/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts @@ -25,7 +25,6 @@ import { ChainModuleBase__factory as ChainModuleBaseFactory } from '../../../typ import { ArbitrumModule__factory as ArbitrumModuleFactory } from '../../../typechain/factories/ArbitrumModule__factory' import { OptimismModule__factory as OptimismModuleFactory } from '../../../typechain/factories/OptimismModule__factory' import { ILogAutomation__factory as ILogAutomationactory } from '../../../typechain/factories/ILogAutomation__factory' -import { IAutomationForwarder__factory as IAutomationForwarderFactory } from '../../../typechain/factories/IAutomationForwarder__factory' import { MockArbSys__factory as MockArbSysFactory } from '../../../typechain/factories/MockArbSys__factory' import { AutomationCompatibleUtils } from '../../../typechain/AutomationCompatibleUtils' import { MockArbGasInfo } from '../../../typechain/MockArbGasInfo' @@ -5423,62 +5422,6 @@ describe('AutomationRegistry2_3', () => { }) }) - describe('#setUpkeepPrivilegeConfig() / #getUpkeepPrivilegeConfig()', () => { - it('reverts when non manager tries to set privilege config', async () => { - await evmRevertCustomError( - registry.connect(payee3).setUpkeepPrivilegeConfig(upkeepId, '0x1234'), - registry, - 'OnlyCallableByUpkeepPrivilegeManager', - ) - }) - - it('returns empty bytes for upkeep privilege config before setting', async () => { - const cfg = await registry.getUpkeepPrivilegeConfig(upkeepId) - assert.equal(cfg, '0x') - }) - - it('allows upkeep manager to set privilege config', async () => { - const tx = await registry - .connect(personas.Norbert) - .setUpkeepPrivilegeConfig(upkeepId, '0x1234') - await expect(tx) - .to.emit(registry, 'UpkeepPrivilegeConfigSet') - .withArgs(upkeepId, '0x1234') - - const cfg = await registry.getUpkeepPrivilegeConfig(upkeepId) - assert.equal(cfg, '0x1234') - }) - }) - - describe('#setAdminPrivilegeConfig() / #getAdminPrivilegeConfig()', () => { - const admin = randomAddress() - - it('reverts when non manager tries to set privilege config', async () => { - await evmRevertCustomError( - registry.connect(payee3).setAdminPrivilegeConfig(admin, '0x1234'), - registry, - 'OnlyCallableByUpkeepPrivilegeManager', - ) - }) - - it('returns empty bytes for upkeep privilege config before setting', async () => { - const cfg = await registry.getAdminPrivilegeConfig(admin) - assert.equal(cfg, '0x') - }) - - it('allows upkeep manager to set privilege config', async () => { - const tx = await registry - .connect(personas.Norbert) - .setAdminPrivilegeConfig(admin, '0x1234') - await expect(tx) - .to.emit(registry, 'AdminPrivilegeConfigSet') - .withArgs(admin, '0x1234') - - const cfg = await registry.getAdminPrivilegeConfig(admin) - assert.equal(cfg, '0x1234') - }) - }) - describe('transmitterPremiumSplit [ @skip-coverage ]', () => { beforeEach(async () => { await linkToken.connect(owner).approve(registry.address, toWei('100')) From 7f3ccc039b4b134a9bd85bd9106549c56c9fd697 Mon Sep 17 00:00:00 2001 From: FelixFan1992 Date: Fri, 26 Jul 2024 14:41:58 -0400 Subject: [PATCH 05/13] tests for upkeep admin --- .../test/v2_3/AutomationRegistry2_3.t.sol | 100 ++++++++++++++ .../automation/AutomationRegistry2_3.test.ts | 123 ------------------ 2 files changed, 100 insertions(+), 123 deletions(-) diff --git a/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol b/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol index 460c7d2fdb6..f8789e15980 100644 --- a/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol +++ b/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol @@ -2223,3 +2223,103 @@ contract SetAdminPrivilegeConfig is SetUp { assertEq(cfg, hex"1233"); } } + +contract TransferUpkeepAdmin is SetUp { + event UpkeepAdminTransferRequested(uint256 indexed id, address indexed from, address indexed to); + + function test_RevertsWhen_NotCalledByAdmin() external { + vm.startPrank(STRANGER); + + vm.expectRevert(Registry.OnlyCallableByAdmin.selector); + registry.transferUpkeepAdmin(linkUpkeepID, randomAddress()); + } + + function test_RevertsWhen_TransferToSelf() external { + vm.startPrank(UPKEEP_ADMIN); + + vm.expectRevert(Registry.ValueNotChanged.selector); + registry.transferUpkeepAdmin(linkUpkeepID, UPKEEP_ADMIN); + } + + function test_RevertsWhen_UpkeepCanceled() external { + vm.startPrank(UPKEEP_ADMIN); + + registry.cancelUpkeep(linkUpkeepID); + + vm.expectRevert(Registry.UpkeepCancelled.selector); + registry.transferUpkeepAdmin(linkUpkeepID, randomAddress()); + } + + function test_DoesNotChangeUpkeepAdmin() external { + vm.startPrank(UPKEEP_ADMIN); + registry.transferUpkeepAdmin(linkUpkeepID, randomAddress()); + + assertEq(registry.getUpkeep(linkUpkeepID).admin, UPKEEP_ADMIN); + } + + function test_EmitEvent_CalledByAdmin() external { + vm.startPrank(UPKEEP_ADMIN); + address newAdmin = randomAddress(); + + vm.expectEmit(); + emit UpkeepAdminTransferRequested(linkUpkeepID, UPKEEP_ADMIN, newAdmin); + registry.transferUpkeepAdmin(linkUpkeepID, newAdmin); + + // transferring to the same propose admin won't yield another event + vm.recordLogs(); + registry.transferUpkeepAdmin(linkUpkeepID, newAdmin); + Vm.Log[] memory entries = vm.getRecordedLogs(); + } + + function test_CancelTransfer_ByTransferToEmptyAddress() external { + vm.startPrank(UPKEEP_ADMIN); + address newAdmin = randomAddress(); + + vm.expectEmit(); + emit UpkeepAdminTransferRequested(linkUpkeepID, UPKEEP_ADMIN, newAdmin); + registry.transferUpkeepAdmin(linkUpkeepID, newAdmin); + + vm.expectEmit(); + emit UpkeepAdminTransferRequested(linkUpkeepID, UPKEEP_ADMIN, address(0)); + registry.transferUpkeepAdmin(linkUpkeepID, address(0)); + } +} + +contract AcceptUpkeepAdmin is SetUp { + event UpkeepAdminTransferred(uint256 indexed id, address indexed from, address indexed to); + + function test_RevertsWhen_NotCalledByProposedAdmin() external { + vm.startPrank(UPKEEP_ADMIN); + address newAdmin = randomAddress(); + registry.transferUpkeepAdmin(linkUpkeepID, newAdmin); + + vm.startPrank(STRANGER); + vm.expectRevert(Registry.OnlyCallableByProposedAdmin.selector); + registry.acceptUpkeepAdmin(linkUpkeepID); + } + + function test_RevertsWhen_UpkeepCanceled() external { + vm.startPrank(UPKEEP_ADMIN); + address newAdmin = randomAddress(); + registry.transferUpkeepAdmin(linkUpkeepID, newAdmin); + + registry.cancelUpkeep(linkUpkeepID); + + vm.startPrank(newAdmin); + vm.expectRevert(Registry.UpkeepCancelled.selector); + registry.acceptUpkeepAdmin(linkUpkeepID); + } + + function test_UpkeepAdminChanged() external { + vm.startPrank(UPKEEP_ADMIN); + address newAdmin = randomAddress(); + registry.transferUpkeepAdmin(linkUpkeepID, newAdmin); + + vm.startPrank(newAdmin); + vm.expectEmit(); + emit UpkeepAdminTransferred(linkUpkeepID, UPKEEP_ADMIN, newAdmin); + registry.acceptUpkeepAdmin(linkUpkeepID); + + assertEq(newAdmin, registry.getUpkeep(linkUpkeepID).admin); + } +} diff --git a/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts b/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts index 47c901c8bd2..3c4f071ee23 100644 --- a/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts +++ b/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts @@ -4550,129 +4550,6 @@ describe('AutomationRegistry2_3', () => { }) }) - describe('#transferUpkeepAdmin', () => { - it('reverts when called by anyone but the current upkeep admin', async () => { - await evmRevertCustomError( - registry - .connect(payee1) - .transferUpkeepAdmin(upkeepId, await payee2.getAddress()), - registry, - 'OnlyCallableByAdmin', - ) - }) - - it('reverts when transferring to self', async () => { - await evmRevertCustomError( - registry - .connect(admin) - .transferUpkeepAdmin(upkeepId, await admin.getAddress()), - registry, - 'ValueNotChanged', - ) - }) - - it('reverts when the upkeep is cancelled', async () => { - await registry.connect(admin).cancelUpkeep(upkeepId) - - await evmRevertCustomError( - registry - .connect(admin) - .transferUpkeepAdmin(upkeepId, await keeper1.getAddress()), - registry, - 'UpkeepCancelled', - ) - }) - - it('allows cancelling transfer by reverting to zero address', async () => { - await registry - .connect(admin) - .transferUpkeepAdmin(upkeepId, await payee1.getAddress()) - const tx = await registry - .connect(admin) - .transferUpkeepAdmin(upkeepId, ethers.constants.AddressZero) - - await expect(tx) - .to.emit(registry, 'UpkeepAdminTransferRequested') - .withArgs( - upkeepId, - await admin.getAddress(), - ethers.constants.AddressZero, - ) - }) - - it('does not change the upkeep admin', async () => { - await registry - .connect(admin) - .transferUpkeepAdmin(upkeepId, await payee1.getAddress()) - - const upkeep = await registry.getUpkeep(upkeepId) - assert.equal(await admin.getAddress(), upkeep.admin) - }) - - it('emits an event announcing the new upkeep admin', async () => { - const tx = await registry - .connect(admin) - .transferUpkeepAdmin(upkeepId, await payee1.getAddress()) - - await expect(tx) - .to.emit(registry, 'UpkeepAdminTransferRequested') - .withArgs(upkeepId, await admin.getAddress(), await payee1.getAddress()) - }) - - it('does not emit an event when called with the same proposed upkeep admin', async () => { - await registry - .connect(admin) - .transferUpkeepAdmin(upkeepId, await payee1.getAddress()) - - const tx = await registry - .connect(admin) - .transferUpkeepAdmin(upkeepId, await payee1.getAddress()) - const receipt = await tx.wait() - assert.equal(receipt.logs.length, 0) - }) - }) - - describe('#acceptUpkeepAdmin', () => { - beforeEach(async () => { - // Start admin transfer to payee1 - await registry - .connect(admin) - .transferUpkeepAdmin(upkeepId, await payee1.getAddress()) - }) - - it('reverts when not called by the proposed upkeep admin', async () => { - await evmRevertCustomError( - registry.connect(payee2).acceptUpkeepAdmin(upkeepId), - registry, - 'OnlyCallableByProposedAdmin', - ) - }) - - it('reverts when the upkeep is cancelled', async () => { - await registry.connect(admin).cancelUpkeep(upkeepId) - - await evmRevertCustomError( - registry.connect(payee1).acceptUpkeepAdmin(upkeepId), - registry, - 'UpkeepCancelled', - ) - }) - - it('does change the admin', async () => { - await registry.connect(payee1).acceptUpkeepAdmin(upkeepId) - - const upkeep = await registry.getUpkeep(upkeepId) - assert.equal(await payee1.getAddress(), upkeep.admin) - }) - - it('emits an event announcing the new upkeep admin', async () => { - const tx = await registry.connect(payee1).acceptUpkeepAdmin(upkeepId) - await expect(tx) - .to.emit(registry, 'UpkeepAdminTransferred') - .withArgs(upkeepId, await admin.getAddress(), await payee1.getAddress()) - }) - }) - describe('#withdrawOwnerFunds', () => { it('can only be called by finance admin', async () => { await evmRevertCustomError( From 28dac6aa6a3dd92a75d7575e5c983e6dfa368a9b Mon Sep 17 00:00:00 2001 From: FelixFan1992 Date: Fri, 26 Jul 2024 15:13:26 -0400 Subject: [PATCH 06/13] tests for pause / unpause upkeeps --- .../test/v2_3/AutomationRegistry2_3.t.sol | 93 +++++++++++++++ .../automation/AutomationRegistry2_3.test.ts | 112 ------------------ 2 files changed, 93 insertions(+), 112 deletions(-) diff --git a/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol b/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol index f8789e15980..cb70abc110e 100644 --- a/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol +++ b/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol @@ -2323,3 +2323,96 @@ contract AcceptUpkeepAdmin is SetUp { assertEq(newAdmin, registry.getUpkeep(linkUpkeepID).admin); } } + +contract PauseUpkeep is SetUp { + event UpkeepPaused(uint256 indexed id); + + function test_RevertsWhen_NotCalledByUpkeepAdmin() external { + vm.startPrank(STRANGER); + + vm.expectRevert(Registry.OnlyCallableByAdmin.selector); + registry.pauseUpkeep(linkUpkeepID); + } + + function test_RevertsWhen_InvalidUpkeepId() external { + vm.startPrank(UPKEEP_ADMIN); + + vm.expectRevert(Registry.OnlyCallableByAdmin.selector); + registry.pauseUpkeep(linkUpkeepID + 1); + } + + function test_RevertsWhen_UpkeepAlreadyCanceled() external { + vm.startPrank(UPKEEP_ADMIN); + registry.cancelUpkeep(linkUpkeepID); + + vm.expectRevert(Registry.UpkeepCancelled.selector); + registry.pauseUpkeep(linkUpkeepID); + } + + function test_RevertsWhen_UpkeepAlreadyPaused() external { + vm.startPrank(UPKEEP_ADMIN); + registry.pauseUpkeep(linkUpkeepID); + + vm.expectRevert(Registry.OnlyUnpausedUpkeep.selector); + registry.pauseUpkeep(linkUpkeepID); + } + + function test_EmitEvent_CalledByAdmin() external { + vm.startPrank(UPKEEP_ADMIN); + + vm.expectEmit(); + emit UpkeepPaused(linkUpkeepID); + registry.pauseUpkeep(linkUpkeepID); + } +} + +contract UnpauseUpkeep is SetUp { + event UpkeepUnpaused(uint256 indexed id); + + function test_RevertsWhen_InvalidUpkeepId() external { + vm.startPrank(UPKEEP_ADMIN); + + vm.expectRevert(Registry.OnlyCallableByAdmin.selector); + registry.unpauseUpkeep(linkUpkeepID + 1); + } + + function test_RevertsWhen_UpkeepIsNotPaused() external { + vm.startPrank(UPKEEP_ADMIN); + + vm.expectRevert(Registry.OnlyPausedUpkeep.selector); + registry.unpauseUpkeep(linkUpkeepID); + } + + function test_RevertsWhen_UpkeepAlreadyCanceled() external { + vm.startPrank(UPKEEP_ADMIN); + registry.pauseUpkeep(linkUpkeepID); + + registry.cancelUpkeep(linkUpkeepID); + + vm.expectRevert(Registry.UpkeepCancelled.selector); + registry.unpauseUpkeep(linkUpkeepID); + } + + function test_RevertsWhen_NotCalledByUpkeepAdmin() external { + vm.startPrank(UPKEEP_ADMIN); + registry.pauseUpkeep(linkUpkeepID); + + vm.startPrank(STRANGER); + vm.expectRevert(Registry.OnlyCallableByAdmin.selector); + registry.unpauseUpkeep(linkUpkeepID); + } + + function test_UnpauseUpkeep_CalledByUpkeepAdmin() external { + vm.startPrank(UPKEEP_ADMIN); + registry.pauseUpkeep(linkUpkeepID); + + uint256[] memory ids1 = registry.getActiveUpkeepIDs(0, 0); + + vm.expectEmit(); + emit UpkeepUnpaused(linkUpkeepID); + registry.unpauseUpkeep(linkUpkeepID); + + uint256[] memory ids2 = registry.getActiveUpkeepIDs(0, 0); + assertEq(ids1.length + 1, ids2.length); + } +} diff --git a/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts b/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts index 3c4f071ee23..d56739a470a 100644 --- a/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts +++ b/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts @@ -4224,118 +4224,6 @@ describe('AutomationRegistry2_3', () => { }) }) - describe('#pauseUpkeep', () => { - it('reverts if the registration does not exist', async () => { - await evmRevertCustomError( - registry.connect(keeper1).pauseUpkeep(upkeepId.add(1)), - registry, - 'OnlyCallableByAdmin', - ) - }) - - it('reverts if the upkeep is already canceled', async () => { - await registry.connect(admin).cancelUpkeep(upkeepId) - - await evmRevertCustomError( - registry.connect(admin).pauseUpkeep(upkeepId), - registry, - 'UpkeepCancelled', - ) - }) - - it('reverts if the upkeep is already paused', async () => { - await registry.connect(admin).pauseUpkeep(upkeepId) - - await evmRevertCustomError( - registry.connect(admin).pauseUpkeep(upkeepId), - registry, - 'OnlyUnpausedUpkeep', - ) - }) - - it('reverts if the caller is not the upkeep admin', async () => { - await evmRevertCustomError( - registry.connect(keeper1).pauseUpkeep(upkeepId), - registry, - 'OnlyCallableByAdmin', - ) - }) - - it('pauses the upkeep and emits an event', async () => { - const tx = await registry.connect(admin).pauseUpkeep(upkeepId) - await expect(tx).to.emit(registry, 'UpkeepPaused').withArgs(upkeepId) - - const registration = await registry.getUpkeep(upkeepId) - assert.equal(registration.paused, true) - }) - }) - - describe('#unpauseUpkeep', () => { - it('reverts if the registration does not exist', async () => { - await evmRevertCustomError( - registry.connect(keeper1).unpauseUpkeep(upkeepId.add(1)), - registry, - 'OnlyCallableByAdmin', - ) - }) - - it('reverts if the upkeep is already canceled', async () => { - await registry.connect(owner).cancelUpkeep(upkeepId) - - await evmRevertCustomError( - registry.connect(admin).unpauseUpkeep(upkeepId), - registry, - 'UpkeepCancelled', - ) - }) - - it('marks the contract as paused', async () => { - assert.isFalse((await registry.getState()).state.paused) - - await registry.connect(owner).pause() - - assert.isTrue((await registry.getState()).state.paused) - }) - - it('reverts if the upkeep is not paused', async () => { - await evmRevertCustomError( - registry.connect(admin).unpauseUpkeep(upkeepId), - registry, - 'OnlyPausedUpkeep', - ) - }) - - it('reverts if the caller is not the upkeep admin', async () => { - await registry.connect(admin).pauseUpkeep(upkeepId) - - const registration = await registry.getUpkeep(upkeepId) - - assert.equal(registration.paused, true) - - await evmRevertCustomError( - registry.connect(keeper1).unpauseUpkeep(upkeepId), - registry, - 'OnlyCallableByAdmin', - ) - }) - - it('unpauses the upkeep and emits an event', async () => { - const originalCount = (await registry.getActiveUpkeepIDs(0, 0)).length - - await registry.connect(admin).pauseUpkeep(upkeepId) - - const tx = await registry.connect(admin).unpauseUpkeep(upkeepId) - - await expect(tx).to.emit(registry, 'UpkeepUnpaused').withArgs(upkeepId) - - const registration = await registry.getUpkeep(upkeepId) - assert.equal(registration.paused, false) - - const upkeepIds = await registry.getActiveUpkeepIDs(0, 0) - assert.equal(upkeepIds.length, originalCount) - }) - }) - describe('#setUpkeepCheckData', () => { it('reverts if the registration does not exist', async () => { await evmRevertCustomError( From a8f0f4abdd6884e137ed625ff1e8872cfed1a458 Mon Sep 17 00:00:00 2001 From: FelixFan1992 Date: Fri, 26 Jul 2024 15:37:16 -0400 Subject: [PATCH 07/13] tests for upkeep configs --- .../test/v2_3/AutomationRegistry2_3.t.sol | 174 ++++++++++++++ .../automation/AutomationRegistry2_3.test.ts | 214 ------------------ 2 files changed, 174 insertions(+), 214 deletions(-) diff --git a/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol b/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol index cb70abc110e..772668da53a 100644 --- a/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol +++ b/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol @@ -2416,3 +2416,177 @@ contract UnpauseUpkeep is SetUp { assertEq(ids1.length + 1, ids2.length); } } + +contract SetUpkeepCheckData is SetUp { + event UpkeepCheckDataSet(uint256 indexed id, bytes newCheckData); + + function test_RevertsWhen_InvalidUpkeepId() external { + vm.startPrank(UPKEEP_ADMIN); + + vm.expectRevert(Registry.OnlyCallableByAdmin.selector); + registry.setUpkeepCheckData(linkUpkeepID + 1, hex"1234"); + } + + function test_RevertsWhen_UpkeepAlreadyCanceled() external { + vm.startPrank(UPKEEP_ADMIN); + registry.cancelUpkeep(linkUpkeepID); + + vm.expectRevert(Registry.UpkeepCancelled.selector); + registry.setUpkeepCheckData(linkUpkeepID, hex"1234"); + } + + function test_RevertsWhen_NewCheckDataTooLarge() external { + vm.startPrank(UPKEEP_ADMIN); + + vm.expectRevert(Registry.CheckDataExceedsLimit.selector); + registry.setUpkeepCheckData(linkUpkeepID, new bytes(10_000)); + } + + function test_RevertsWhen_NotCalledByAdmin() external { + vm.startPrank(STRANGER); + + vm.expectRevert(Registry.OnlyCallableByAdmin.selector); + registry.setUpkeepCheckData(linkUpkeepID, hex"1234"); + } + + function test_UpdateOffchainConfig_CalledByAdmin() external { + vm.startPrank(UPKEEP_ADMIN); + + vm.expectEmit(); + emit UpkeepCheckDataSet(linkUpkeepID, hex"1234"); + registry.setUpkeepCheckData(linkUpkeepID, hex"1234"); + + assertEq(registry.getUpkeep(linkUpkeepID).checkData, hex"1234"); + } + + function test_UpdateOffchainConfigOnPausedUpkeep_CalledByAdmin() external { + vm.startPrank(UPKEEP_ADMIN); + + registry.pauseUpkeep(linkUpkeepID); + + vm.expectEmit(); + emit UpkeepCheckDataSet(linkUpkeepID, hex"1234"); + registry.setUpkeepCheckData(linkUpkeepID, hex"1234"); + + assertTrue(registry.getUpkeep(linkUpkeepID).paused); + assertEq(registry.getUpkeep(linkUpkeepID).checkData, hex"1234"); + } +} + +contract SetUpkeepGasLimit is SetUp { + event UpkeepGasLimitSet(uint256 indexed id, uint96 gasLimit); + + function test_RevertsWhen_InvalidUpkeepId() external { + vm.startPrank(UPKEEP_ADMIN); + + vm.expectRevert(Registry.OnlyCallableByAdmin.selector); + registry.setUpkeepGasLimit(linkUpkeepID + 1, 1230000); + } + + function test_RevertsWhen_UpkeepAlreadyCanceled() external { + vm.startPrank(UPKEEP_ADMIN); + registry.cancelUpkeep(linkUpkeepID); + + vm.expectRevert(Registry.UpkeepCancelled.selector); + registry.setUpkeepGasLimit(linkUpkeepID, 1230000); + } + + function test_RevertsWhen_NewGasLimitOutOfRange() external { + vm.startPrank(UPKEEP_ADMIN); + + vm.expectRevert(Registry.GasLimitOutsideRange.selector); + registry.setUpkeepGasLimit(linkUpkeepID, 300); + + vm.expectRevert(Registry.GasLimitOutsideRange.selector); + registry.setUpkeepGasLimit(linkUpkeepID, 3000000000); + } + + function test_RevertsWhen_NotCalledByAdmin() external { + vm.startPrank(STRANGER); + + vm.expectRevert(Registry.OnlyCallableByAdmin.selector); + registry.setUpkeepGasLimit(linkUpkeepID, 1230000); + } + + function test_UpdateGasLimit_CalledByAdmin() external { + vm.startPrank(UPKEEP_ADMIN); + + vm.expectEmit(); + emit UpkeepGasLimitSet(linkUpkeepID, 1230000); + registry.setUpkeepGasLimit(linkUpkeepID, 1230000); + + assertEq(registry.getUpkeep(linkUpkeepID).performGas, 1230000); + } +} + +contract SetUpkeepOffchainConfig is SetUp { + event UpkeepOffchainConfigSet(uint256 indexed id, bytes offchainConfig); + + function test_RevertsWhen_InvalidUpkeepId() external { + vm.startPrank(UPKEEP_ADMIN); + + vm.expectRevert(Registry.OnlyCallableByAdmin.selector); + registry.setUpkeepOffchainConfig(linkUpkeepID + 1, hex"1233"); + } + + function test_RevertsWhen_UpkeepAlreadyCanceled() external { + vm.startPrank(UPKEEP_ADMIN); + registry.cancelUpkeep(linkUpkeepID); + + vm.expectRevert(Registry.UpkeepCancelled.selector); + registry.setUpkeepOffchainConfig(linkUpkeepID, hex"1234"); + } + + function test_RevertsWhen_NotCalledByAdmin() external { + vm.startPrank(STRANGER); + + vm.expectRevert(Registry.OnlyCallableByAdmin.selector); + registry.setUpkeepOffchainConfig(linkUpkeepID, hex"1234"); + } + + function test_UpdateOffchainConfig_CalledByAdmin() external { + vm.startPrank(UPKEEP_ADMIN); + + vm.expectEmit(); + emit UpkeepOffchainConfigSet(linkUpkeepID, hex"1234"); + registry.setUpkeepOffchainConfig(linkUpkeepID, hex"1234"); + + assertEq(registry.getUpkeep(linkUpkeepID).offchainConfig, hex"1234"); + } +} + +contract SetUpkeepTriggerConfig is SetUp { + event UpkeepTriggerConfigSet(uint256 indexed id, bytes triggerConfig); + + function test_RevertsWhen_InvalidUpkeepId() external { + vm.startPrank(UPKEEP_ADMIN); + + vm.expectRevert(Registry.OnlyCallableByAdmin.selector); + registry.setUpkeepTriggerConfig(linkUpkeepID + 1, hex"1233"); + } + + function test_RevertsWhen_UpkeepAlreadyCanceled() external { + vm.startPrank(UPKEEP_ADMIN); + registry.cancelUpkeep(linkUpkeepID); + + vm.expectRevert(Registry.UpkeepCancelled.selector); + registry.setUpkeepTriggerConfig(linkUpkeepID, hex"1234"); + } + + function test_RevertsWhen_NotCalledByAdmin() external { + vm.startPrank(STRANGER); + + vm.expectRevert(Registry.OnlyCallableByAdmin.selector); + registry.setUpkeepTriggerConfig(linkUpkeepID, hex"1234"); + } + + function test_UpdateTriggerConfig_CalledByAdmin() external { + vm.startPrank(UPKEEP_ADMIN); + + vm.expectEmit(); + emit UpkeepTriggerConfigSet(linkUpkeepID, hex"1234"); + registry.setUpkeepTriggerConfig(linkUpkeepID, hex"1234"); + + assertEq(registry.getUpkeepTriggerConfig(linkUpkeepID), hex"1234"); + } +} diff --git a/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts b/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts index d56739a470a..5712af6d463 100644 --- a/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts +++ b/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts @@ -4224,220 +4224,6 @@ describe('AutomationRegistry2_3', () => { }) }) - describe('#setUpkeepCheckData', () => { - it('reverts if the registration does not exist', async () => { - await evmRevertCustomError( - registry - .connect(keeper1) - .setUpkeepCheckData(upkeepId.add(1), randomBytes), - registry, - 'OnlyCallableByAdmin', - ) - }) - - it('reverts if the caller is not upkeep admin', async () => { - await evmRevertCustomError( - registry.connect(keeper1).setUpkeepCheckData(upkeepId, randomBytes), - registry, - 'OnlyCallableByAdmin', - ) - }) - - it('reverts if the upkeep is cancelled', async () => { - await registry.connect(admin).cancelUpkeep(upkeepId) - - await evmRevertCustomError( - registry.connect(admin).setUpkeepCheckData(upkeepId, randomBytes), - registry, - 'UpkeepCancelled', - ) - }) - - it('is allowed to update on paused upkeep', async () => { - await registry.connect(admin).pauseUpkeep(upkeepId) - await registry.connect(admin).setUpkeepCheckData(upkeepId, randomBytes) - - const registration = await registry.getUpkeep(upkeepId) - assert.equal(randomBytes, registration.checkData) - }) - - it('reverts if new data exceeds limit', async () => { - let longBytes = '0x' - for (let i = 0; i < 10000; i++) { - longBytes += '1' - } - - await evmRevertCustomError( - registry.connect(admin).setUpkeepCheckData(upkeepId, longBytes), - registry, - 'CheckDataExceedsLimit', - ) - }) - - it('updates the upkeep check data and emits an event', async () => { - const tx = await registry - .connect(admin) - .setUpkeepCheckData(upkeepId, randomBytes) - await expect(tx) - .to.emit(registry, 'UpkeepCheckDataSet') - .withArgs(upkeepId, randomBytes) - - const registration = await registry.getUpkeep(upkeepId) - assert.equal(randomBytes, registration.checkData) - }) - }) - - describe('#setUpkeepGasLimit', () => { - const newGasLimit = BigNumber.from('300000') - - it('reverts if the registration does not exist', async () => { - await evmRevertCustomError( - registry.connect(admin).setUpkeepGasLimit(upkeepId.add(1), newGasLimit), - registry, - 'OnlyCallableByAdmin', - ) - }) - - it('reverts if the upkeep is canceled', async () => { - await registry.connect(admin).cancelUpkeep(upkeepId) - await evmRevertCustomError( - registry.connect(admin).setUpkeepGasLimit(upkeepId, newGasLimit), - registry, - 'UpkeepCancelled', - ) - }) - - it('reverts if called by anyone but the admin', async () => { - await evmRevertCustomError( - registry.connect(owner).setUpkeepGasLimit(upkeepId, newGasLimit), - registry, - 'OnlyCallableByAdmin', - ) - }) - - it('reverts if new gas limit is out of bounds', async () => { - await evmRevertCustomError( - registry - .connect(admin) - .setUpkeepGasLimit(upkeepId, BigNumber.from('100')), - registry, - 'GasLimitOutsideRange', - ) - await evmRevertCustomError( - registry - .connect(admin) - .setUpkeepGasLimit(upkeepId, BigNumber.from('6000000')), - registry, - 'GasLimitOutsideRange', - ) - }) - - it('updates the gas limit successfully', async () => { - const initialGasLimit = (await registry.getUpkeep(upkeepId)).performGas - assert.equal(initialGasLimit, performGas.toNumber()) - await registry.connect(admin).setUpkeepGasLimit(upkeepId, newGasLimit) - const updatedGasLimit = (await registry.getUpkeep(upkeepId)).performGas - assert.equal(updatedGasLimit, newGasLimit.toNumber()) - }) - - it('emits a log', async () => { - const tx = await registry - .connect(admin) - .setUpkeepGasLimit(upkeepId, newGasLimit) - await expect(tx) - .to.emit(registry, 'UpkeepGasLimitSet') - .withArgs(upkeepId, newGasLimit) - }) - }) - - describe('#setUpkeepOffchainConfig', () => { - const newConfig = '0xc0ffeec0ffee' - - it('reverts if the registration does not exist', async () => { - await evmRevertCustomError( - registry - .connect(admin) - .setUpkeepOffchainConfig(upkeepId.add(1), newConfig), - registry, - 'OnlyCallableByAdmin', - ) - }) - - it('reverts if the upkeep is canceled', async () => { - await registry.connect(admin).cancelUpkeep(upkeepId) - await evmRevertCustomError( - registry.connect(admin).setUpkeepOffchainConfig(upkeepId, newConfig), - registry, - 'UpkeepCancelled', - ) - }) - - it('reverts if called by anyone but the admin', async () => { - await evmRevertCustomError( - registry.connect(owner).setUpkeepOffchainConfig(upkeepId, newConfig), - registry, - 'OnlyCallableByAdmin', - ) - }) - - it('updates the config successfully', async () => { - const initialConfig = (await registry.getUpkeep(upkeepId)).offchainConfig - assert.equal(initialConfig, '0x') - await registry.connect(admin).setUpkeepOffchainConfig(upkeepId, newConfig) - const updatedConfig = (await registry.getUpkeep(upkeepId)).offchainConfig - assert.equal(newConfig, updatedConfig) - }) - - it('emits a log', async () => { - const tx = await registry - .connect(admin) - .setUpkeepOffchainConfig(upkeepId, newConfig) - await expect(tx) - .to.emit(registry, 'UpkeepOffchainConfigSet') - .withArgs(upkeepId, newConfig) - }) - }) - - describe('#setUpkeepTriggerConfig', () => { - const newConfig = '0xdeadbeef' - - it('reverts if the registration does not exist', async () => { - await evmRevertCustomError( - registry - .connect(admin) - .setUpkeepTriggerConfig(upkeepId.add(1), newConfig), - registry, - 'OnlyCallableByAdmin', - ) - }) - - it('reverts if the upkeep is canceled', async () => { - await registry.connect(admin).cancelUpkeep(upkeepId) - await evmRevertCustomError( - registry.connect(admin).setUpkeepTriggerConfig(upkeepId, newConfig), - registry, - 'UpkeepCancelled', - ) - }) - - it('reverts if called by anyone but the admin', async () => { - await evmRevertCustomError( - registry.connect(owner).setUpkeepTriggerConfig(upkeepId, newConfig), - registry, - 'OnlyCallableByAdmin', - ) - }) - - it('emits a log', async () => { - const tx = await registry - .connect(admin) - .setUpkeepTriggerConfig(upkeepId, newConfig) - await expect(tx) - .to.emit(registry, 'UpkeepTriggerConfigSet') - .withArgs(upkeepId, newConfig) - }) - }) - describe('#withdrawOwnerFunds', () => { it('can only be called by finance admin', async () => { await evmRevertCustomError( From 9fbe36e6f4a2e39b512ad34861fdad7fa1e8319f Mon Sep 17 00:00:00 2001 From: FelixFan1992 Date: Fri, 26 Jul 2024 16:40:03 -0400 Subject: [PATCH 08/13] tests for payees --- .../test/v2_3/AutomationRegistry2_3.t.sol | 106 ++++++++++++- .../automation/AutomationRegistry2_3.test.ts | 142 +----------------- 2 files changed, 102 insertions(+), 146 deletions(-) diff --git a/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol b/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol index 772668da53a..ac09c700c17 100644 --- a/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol +++ b/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol @@ -22,11 +22,11 @@ contract SetUp is BaseTest { AutomationRegistryBase2_3.OnchainConfig internal config; bytes internal constant offchainConfigBytes = abi.encode(1234, ZERO_ADDRESS); - uint256 linkUpkeepID; - uint256 linkUpkeepID2; // 2 upkeeps use the same billing token (LINK) to test migration scenario - uint256 usdUpkeepID18; // 1 upkeep uses ERC20 token with 18 decimals - uint256 usdUpkeepID6; // 1 upkeep uses ERC20 token with 6 decimals - uint256 nativeUpkeepID; + uint256 internal linkUpkeepID; + uint256 internal linkUpkeepID2; // 2 upkeeps use the same billing token (LINK) to test migration scenario + uint256 internal usdUpkeepID18; // 1 upkeep uses ERC20 token with 18 decimals + uint256 internal usdUpkeepID6; // 1 upkeep uses ERC20 token with 6 decimals + uint256 internal nativeUpkeepID; function setUp() public virtual override { super.setUp(); @@ -2269,6 +2269,7 @@ contract TransferUpkeepAdmin is SetUp { vm.recordLogs(); registry.transferUpkeepAdmin(linkUpkeepID, newAdmin); Vm.Log[] memory entries = vm.getRecordedLogs(); + assertEq(0, entries.length); } function test_CancelTransfer_ByTransferToEmptyAddress() external { @@ -2590,3 +2591,98 @@ contract SetUpkeepTriggerConfig is SetUp { assertEq(registry.getUpkeepTriggerConfig(linkUpkeepID), hex"1234"); } } + +contract TransferPayeeship is SetUp { + event PayeeshipTransferRequested(address indexed transmitter, address indexed from, address indexed to); + + function test_RevertsWhen_NotCalledByPayee() external { + vm.startPrank(STRANGER); + + vm.expectRevert(Registry.OnlyCallableByPayee.selector); + registry.transferPayeeship(TRANSMITTERS[0], randomAddress()); + } + + function test_RevertsWhen_TransferToSelf() external { + vm.startPrank(PAYEES[0]); + + vm.expectRevert(Registry.ValueNotChanged.selector); + registry.transferPayeeship(TRANSMITTERS[0], PAYEES[0]); + } + + function test_Transfer_DoesNotChangePayee() external { + vm.startPrank(PAYEES[0]); + + registry.transferPayeeship(TRANSMITTERS[0], randomAddress()); + + (, , , , address payee) = registry.getTransmitterInfo(TRANSMITTERS[0]); + assertEq(PAYEES[0], payee); + } + + function test_EmitEvent_CalledByPayee() external { + vm.startPrank(PAYEES[0]); + address newPayee = randomAddress(); + + vm.expectEmit(); + emit PayeeshipTransferRequested(TRANSMITTERS[0], PAYEES[0], newPayee); + registry.transferPayeeship(TRANSMITTERS[0], newPayee); + + // transferring to the same propose payee won't yield another event + vm.recordLogs(); + registry.transferPayeeship(TRANSMITTERS[0], newPayee); + Vm.Log[] memory entries = vm.getRecordedLogs(); + assertEq(0, entries.length); + } +} + +contract AcceptPayeeship is SetUp { + event PayeeshipTransferred(address indexed transmitter, address indexed from, address indexed to); + + function test_RevertsWhen_NotCalledByProposedPayee() external { + vm.startPrank(PAYEES[0]); + address newPayee = randomAddress(); + registry.transferPayeeship(TRANSMITTERS[0], newPayee); + + vm.startPrank(STRANGER); + vm.expectRevert(Registry.OnlyCallableByProposedPayee.selector); + registry.acceptPayeeship(TRANSMITTERS[0]); + } + + function test_PayeeChanged() external { + vm.startPrank(PAYEES[0]); + address newPayee = randomAddress(); + registry.transferPayeeship(TRANSMITTERS[0], newPayee); + + vm.startPrank(newPayee); + vm.expectEmit(); + emit PayeeshipTransferred(TRANSMITTERS[0], PAYEES[0], newPayee); + registry.acceptPayeeship(TRANSMITTERS[0]); + + (, , , , address payee) = registry.getTransmitterInfo(TRANSMITTERS[0]); + assertEq(newPayee, payee); + } +} + +contract SetPayees is SetUp { + function test_RevertsWhen_NotCalledByOwner() external { + vm.startPrank(STRANGER); + + vm.expectRevert(bytes("Only callable by owner")); + registry.setPayees(NEW_PAYEES); + } + + function test_RevertsWhen_PayeesLengthError() external { + vm.startPrank(registry.owner()); + + address[] memory payees = new address[](5); + vm.expectRevert(Registry.ParameterLengthError.selector); + registry.setPayees(payees); + } + + function test_RevertsWhen_InvalidPayee() external { + vm.startPrank(registry.owner()); + + NEW_PAYEES[0] = address(0); + vm.expectRevert(Registry.InvalidPayee.selector); + registry.setPayees(NEW_PAYEES); + } +} diff --git a/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts b/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts index 5712af6d463..b8420164972 100644 --- a/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts +++ b/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts @@ -4299,119 +4299,6 @@ describe('AutomationRegistry2_3', () => { }) }) - describe('#transferPayeeship', () => { - it('reverts when called by anyone but the current payee', async () => { - await evmRevertCustomError( - registry - .connect(payee2) - .transferPayeeship( - await keeper1.getAddress(), - await payee2.getAddress(), - ), - registry, - 'OnlyCallableByPayee', - ) - }) - - it('reverts when transferring to self', async () => { - await evmRevertCustomError( - registry - .connect(payee1) - .transferPayeeship( - await keeper1.getAddress(), - await payee1.getAddress(), - ), - registry, - 'ValueNotChanged', - ) - }) - - it('does not change the payee', async () => { - await registry - .connect(payee1) - .transferPayeeship( - await keeper1.getAddress(), - await payee2.getAddress(), - ) - - const info = await registry.getTransmitterInfo(await keeper1.getAddress()) - assert.equal(await payee1.getAddress(), info.payee) - }) - - it('emits an event announcing the new payee', async () => { - const tx = await registry - .connect(payee1) - .transferPayeeship( - await keeper1.getAddress(), - await payee2.getAddress(), - ) - await expect(tx) - .to.emit(registry, 'PayeeshipTransferRequested') - .withArgs( - await keeper1.getAddress(), - await payee1.getAddress(), - await payee2.getAddress(), - ) - }) - - it('does not emit an event when called with the same proposal', async () => { - await registry - .connect(payee1) - .transferPayeeship( - await keeper1.getAddress(), - await payee2.getAddress(), - ) - - const tx = await registry - .connect(payee1) - .transferPayeeship( - await keeper1.getAddress(), - await payee2.getAddress(), - ) - const receipt = await tx.wait() - assert.equal(receipt.logs.length, 0) - }) - }) - - describe('#acceptPayeeship', () => { - beforeEach(async () => { - await registry - .connect(payee1) - .transferPayeeship( - await keeper1.getAddress(), - await payee2.getAddress(), - ) - }) - - it('reverts when called by anyone but the proposed payee', async () => { - await evmRevertCustomError( - registry.connect(payee1).acceptPayeeship(await keeper1.getAddress()), - registry, - 'OnlyCallableByProposedPayee', - ) - }) - - it('emits an event announcing the new payee', async () => { - const tx = await registry - .connect(payee2) - .acceptPayeeship(await keeper1.getAddress()) - await expect(tx) - .to.emit(registry, 'PayeeshipTransferred') - .withArgs( - await keeper1.getAddress(), - await payee1.getAddress(), - await payee2.getAddress(), - ) - }) - - it('does change the payee', async () => { - await registry.connect(payee2).acceptPayeeship(await keeper1.getAddress()) - - const info = await registry.getTransmitterInfo(await keeper1.getAddress()) - assert.equal(await payee2.getAddress(), info.payee) - }) - }) - describe('#pause', () => { it('Does not allow transmits when paused', async () => { await registry.connect(owner).pause() @@ -4427,35 +4314,8 @@ describe('AutomationRegistry2_3', () => { describe('#setPayees', () => { const IGNORE_ADDRESS = '0xFFfFfFffFFfffFFfFFfFFFFFffFFFffffFfFFFfF' - it('reverts when not called by the owner', async () => { - await evmRevert( - registry.connect(keeper1).setPayees(payees), - 'Only callable by owner', - ) - }) - - it('reverts with different numbers of payees than transmitters', async () => { - await evmRevertCustomError( - registry.connect(owner).setPayees([...payees, randomAddress()]), - registry, - 'ParameterLengthError', - ) - }) - - it('reverts if the payee is the zero address', async () => { - await blankRegistry.connect(owner).setConfigTypeSafe(...baseConfig) // used to test initial config - - await evmRevertCustomError( - blankRegistry // used to test initial config - .connect(owner) - .setPayees([ethers.constants.AddressZero, ...payees.slice(1)]), - registry, - 'InvalidPayee', - ) - }) - itMaybe( - 'sets the payees when exisitng payees are zero address', + 'sets the payees when existing payees are zero address', async () => { //Initial payees should be zero address await blankRegistry.connect(owner).setConfigTypeSafe(...baseConfig) // used to test initial config From b7c141c6fa42273d206937f5eb13a545236644f6 Mon Sep 17 00:00:00 2001 From: FelixFan1992 Date: Fri, 2 Aug 2024 10:16:55 -0400 Subject: [PATCH 09/13] cancel upkeep tests --- .../test/v2_3/AutomationRegistry2_3.t.sol | 57 ++++++++--------- .../v0.8/automation/test/v2_3/BaseTest.t.sol | 63 ++++++++++--------- .../automation/AutomationRegistry2_3.test.ts | 32 ---------- 3 files changed, 64 insertions(+), 88 deletions(-) diff --git a/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol b/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol index ac09c700c17..532257b17a4 100644 --- a/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol +++ b/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol @@ -349,7 +349,7 @@ contract Withdraw is SetUp { // default is ON_CHAIN mode function test_WithdrawERC20Fees_RevertsWhen_LinkAvailableForPaymentIsNegative() public { - _transmit(usdUpkeepID18, registry); // adds USD token to finance withdrawable, and gives NOPs a LINK balance + _transmit(usdUpkeepID18, registry, bytes4(0)); // adds USD token to finance withdrawable, and gives NOPs a LINK balance require(registry.linkAvailableForPayment() < 0, "linkAvailableForPayment should be negative"); require( registry.getAvailableERC20ForPayment(address(usdToken18)) > 0, @@ -375,7 +375,7 @@ contract Withdraw is SetUp { registry.addFunds(id, 1e20); // manually create a transmit so transmitters earn some rewards - _transmit(id, registry); + _transmit(id, registry, bytes4(0)); require(registry.linkAvailableForPayment() < 0, "linkAvailableForPayment should be negative"); vm.prank(FINANCE_ADMIN); registry.withdrawERC20Fees(address(usdToken18), aMockAddress, 1); // finance can withdraw @@ -1000,7 +1000,7 @@ contract NOPsSettlement is SetUp { registry.addFunds(id, 1e20); // manually create a transmit so transmitters earn some rewards - _transmit(id, registry); + _transmit(id, registry, bytes4(0)); // verify transmitters have positive balances uint256[] memory payments = new uint256[](TRANSMITTERS.length); @@ -1037,7 +1037,7 @@ contract NOPsSettlement is SetUp { vm.startPrank(UPKEEP_ADMIN); vm.roll(100 + block.number); // manually create a transmit so transmitters earn some rewards - _transmit(id, registry); + _transmit(id, registry, bytes4(0)); uint256 erc20ForPayment2 = registry.getAvailableERC20ForPayment(address(usdToken18)); require(erc20ForPayment2 > erc20ForPayment1, "ERC20AvailableForPayment should be greater after another transmit"); @@ -1080,13 +1080,13 @@ contract NOPsSettlement is SetUp { registry.addFunds(id, 1e20); // manually create a transmit so TRANSMITTERS earn some rewards - _transmit(id, registry); + _transmit(id, registry, bytes4(0)); // TRANSMITTERS have positive balance now // configure the registry to use NEW_TRANSMITTERS _configureWithNewTransmitters(registry, registrar); - _transmit(id, registry); + _transmit(id, registry, bytes4(0)); // verify all transmitters have positive balances address[] memory expectedPayees = new address[](6); @@ -1195,7 +1195,7 @@ contract NOPsSettlement is SetUp { registry.addFunds(id, 1e20); // manually create a transmit so transmitters earn some rewards - _transmit(id, registry); + _transmit(id, registry, bytes4(0)); // disable offchain payments _mintLink(address(registry), 1e19); @@ -1235,7 +1235,7 @@ contract NOPsSettlement is SetUp { // manually call transmit so transmitters earn some rewards for (uint256 i = 0; i < 50; i++) { vm.roll(100 + block.number); - _transmit(id, registry); + _transmit(id, registry, bytes4(0)); } // disable offchain payments @@ -1246,7 +1246,7 @@ contract NOPsSettlement is SetUp { // manually call transmit after offchain payment is disabled for (uint256 i = 0; i < 50; i++) { vm.roll(100 + block.number); - _transmit(id, registry); + _transmit(id, registry, bytes4(0)); } // payees should be able to withdraw onchain @@ -1671,7 +1671,7 @@ contract Transmit is SetUp { require(registry.getAvailableERC20ForPayment(address(weth)) == 0, "ERC20AvailableForPayment should be 0"); // do the thing - _transmit(upkeepIDs, registry); + _transmit(upkeepIDs, registry, bytes4(0)); // withdraw-able by the finance team should be positive require( @@ -1726,7 +1726,7 @@ contract Transmit is SetUp { // manually create a transmit vm.recordLogs(); - _transmit(upkeepID, registry); + _transmit(upkeepID, registry, bytes4(0)); Vm.Log[] memory entries = vm.getRecordedLogs(); assertEq(entries.length, 3); @@ -1777,7 +1777,7 @@ contract Transmit is SetUp { // manually create a transmit vm.recordLogs(); - _transmit(upkeepID, registry); + _transmit(upkeepID, registry, bytes4(0)); Vm.Log[] memory entries = vm.getRecordedLogs(); assertEq(entries.length, 3); @@ -2010,13 +2010,12 @@ contract Pause is SetUp { ); } - // function test_revertsWhen_transmitInPausedRegistry() external { - // vm.startPrank(registry.owner()); - // registry.pause(); - // - // vm.expectRevert(Registry.RegistryPaused.selector); - // _transmit(usdUpkeepID18, registry); - // } + function test_revertsWhen_transmitInPausedRegistry() external { + vm.startPrank(registry.owner()); + registry.pause(); + + _transmit(usdUpkeepID18, registry, Registry.RegistryPaused.selector); + } } contract Unpause is SetUp { @@ -2067,31 +2066,35 @@ contract CancelUpkeep is SetUp { vm.startPrank(UPKEEP_ADMIN); registry.cancelUpkeep(linkUpkeepID); - vm.roll(50 + bn); - vm.startPrank(registry.owner()); vm.expectRevert(Registry.UpkeepCancelled.selector); registry.cancelUpkeep(linkUpkeepID); } - function test_RevertsWhen_UpkeepAlreadyCanceled_CalledByAdmin() external { + function test_RevertsWhen_UpkeepAlreadyCanceledByOwner_CalledByAdmin() external { uint256 bn = block.number; + vm.startPrank(registry.owner()); + registry.cancelUpkeep(linkUpkeepID); + vm.startPrank(UPKEEP_ADMIN); + vm.expectRevert(Registry.UpkeepCancelled.selector); registry.cancelUpkeep(linkUpkeepID); + } - vm.roll(10 + bn); + function test_RevertsWhen_UpkeepAlreadyCanceledByAdmin_CalledByAdmin() external { + uint256 bn = block.number; + vm.startPrank(UPKEEP_ADMIN); + registry.cancelUpkeep(linkUpkeepID); vm.expectRevert(Registry.UpkeepCancelled.selector); registry.cancelUpkeep(linkUpkeepID); } - function test_RevertsWhen_UpkeepAlreadyCanceled_CalledByOwner() external { + function test_RevertsWhen_UpkeepAlreadyCanceledByOwner_CalledByOwner() external { uint256 bn = block.number; vm.startPrank(registry.owner()); registry.cancelUpkeep(linkUpkeepID); - vm.roll(10 + bn); - vm.expectRevert(Registry.UpkeepCancelled.selector); registry.cancelUpkeep(linkUpkeepID); } @@ -2101,7 +2104,6 @@ contract CancelUpkeep is SetUp { vm.startPrank(UPKEEP_ADMIN); registry.cancelUpkeep(linkUpkeepID); - vm.roll(10 + bn); uint256 maxValidBlocknumber = uint256(registry.getUpkeep(linkUpkeepID).maxValidBlocknumber); // 50 is the cancellation delay @@ -2113,7 +2115,6 @@ contract CancelUpkeep is SetUp { vm.startPrank(registry.owner()); registry.cancelUpkeep(linkUpkeepID); - vm.roll(10 + bn); uint256 maxValidBlocknumber = uint256(registry.getUpkeep(linkUpkeepID).maxValidBlocknumber); // cancellation by registry owner is immediate and no cancellation delay is applied diff --git a/contracts/src/v0.8/automation/test/v2_3/BaseTest.t.sol b/contracts/src/v0.8/automation/test/v2_3/BaseTest.t.sol index 9016f52c55d..b966a5dee06 100644 --- a/contracts/src/v0.8/automation/test/v2_3/BaseTest.t.sol +++ b/contracts/src/v0.8/automation/test/v2_3/BaseTest.t.sol @@ -356,40 +356,44 @@ contract BaseTest is Test { ); } - function _transmit(uint256 id, Registry registry) internal { + function _transmit(uint256 id, Registry registry, bytes4 selector) internal { uint256[] memory ids = new uint256[](1); ids[0] = id; - _transmit(ids, registry); + _transmit(ids, registry, selector); } - function _transmit(uint256[] memory ids, Registry registry) internal { - uint256[] memory upkeepIds = new uint256[](ids.length); - uint256[] memory gasLimits = new uint256[](ids.length); - bytes[] memory performDatas = new bytes[](ids.length); - bytes[] memory triggers = new bytes[](ids.length); - for (uint256 i = 0; i < ids.length; i++) { - upkeepIds[i] = ids[i]; - gasLimits[i] = registry.getUpkeep(ids[i]).performGas; - performDatas[i] = new bytes(0); - uint8 triggerType = registry.getTriggerType(ids[i]); - if (triggerType == 0) { - triggers[i] = _encodeConditionalTrigger( - AutoBase.ConditionalTrigger(uint32(block.number - 1), blockhash(block.number - 1)) - ); - } else { - revert("not implemented"); + function _transmit(uint256[] memory ids, Registry registry, bytes4 selector) internal { + bytes memory reportBytes; + { + uint256[] memory upkeepIds = new uint256[](ids.length); + uint256[] memory gasLimits = new uint256[](ids.length); + bytes[] memory performDatas = new bytes[](ids.length); + bytes[] memory triggers = new bytes[](ids.length); + for (uint256 i = 0; i < ids.length; i++) { + upkeepIds[i] = ids[i]; + gasLimits[i] = registry.getUpkeep(ids[i]).performGas; + performDatas[i] = new bytes(0); + uint8 triggerType = registry.getTriggerType(ids[i]); + if (triggerType == 0) { + triggers[i] = _encodeConditionalTrigger( + AutoBase.ConditionalTrigger(uint32(block.number - 1), blockhash(block.number - 1)) + ); + } else { + revert("not implemented"); + } } - } - AutoBase.Report memory report = AutoBase.Report( - uint256(1000000000), - uint256(2000000000), - upkeepIds, - gasLimits, - triggers, - performDatas - ); - bytes memory reportBytes = _encodeReport(report); + AutoBase.Report memory report = AutoBase.Report( + uint256(1000000000), + uint256(2000000000), + upkeepIds, + gasLimits, + triggers, + performDatas + ); + + reportBytes = _encodeReport(report); + } (, , bytes32 configDigest) = registry.latestConfigDetails(); bytes32[3] memory reportContext = [configDigest, configDigest, configDigest]; uint256[] memory signerPKs = new uint256[](2); @@ -398,6 +402,9 @@ contract BaseTest is Test { (bytes32[] memory rs, bytes32[] memory ss, bytes32 vs) = _signReport(reportBytes, reportContext, signerPKs); vm.startPrank(TRANSMITTERS[0]); + if (selector != bytes4(0)) { + vm.expectRevert(selector); + } registry.transmit(reportContext, reportBytes, rs, ss, vs); vm.stopPrank(); } diff --git a/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts b/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts index b8420164972..a7acde283fc 100644 --- a/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts +++ b/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts @@ -4299,18 +4299,6 @@ describe('AutomationRegistry2_3', () => { }) }) - describe('#pause', () => { - it('Does not allow transmits when paused', async () => { - await registry.connect(owner).pause() - - await evmRevertCustomError( - getTransmitTx(registry, keeper1, [upkeepId]), - registry, - 'RegistryPaused', - ) - }) - }) - describe('#setPayees', () => { const IGNORE_ADDRESS = '0xFFfFfFffFFfffFFfFFfFFFFFffFFFffffFfFFFfF' @@ -4415,26 +4403,6 @@ describe('AutomationRegistry2_3', () => { // exactly 1 CancelledUpkeepReport log should be emitted assert.equal(cancelledUpkeepReportLogs.length, 1) }) - - describe('when called by the owner when the admin has just canceled', () => { - // eslint-disable-next-line @typescript-eslint/no-unused-vars - // @ts-ignore - let oldExpiration: BigNumber - - beforeEach(async () => { - await registry.connect(admin).cancelUpkeep(upkeepId) - const registration = await registry.getUpkeep(upkeepId) - oldExpiration = registration.maxValidBlocknumber - }) - - it('reverts with proper error', async () => { - await evmRevertCustomError( - registry.connect(owner).cancelUpkeep(upkeepId), - registry, - 'UpkeepCancelled', - ) - }) - }) }) describe('when called by the admin', async () => { From 37dd0924cdac51606d1d419457f41ce123f75db7 Mon Sep 17 00:00:00 2001 From: FelixFan1992 Date: Fri, 2 Aug 2024 10:43:23 -0400 Subject: [PATCH 10/13] update --- .../test/v2_3/AutomationRegistry2_3.t.sol | 14 ++++++++++++++ .../src/v0.8/automation/test/v2_3/BaseTest.t.sol | 1 - 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol b/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol index 532257b17a4..bb1f2237d07 100644 --- a/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol +++ b/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol @@ -790,6 +790,7 @@ contract SetConfig is SetUp { } function testSetConfigOnTransmittersAndPayees() public { + registry.setPayees(PAYEES); AutomationRegistryBase2_3.TransmitterPayeeInfo[] memory transmitterPayeeInfos = registry .getTransmittersWithPayees(); assertEq(transmitterPayeeInfos.length, TRANSMITTERS.length); @@ -975,6 +976,7 @@ contract NOPsSettlement is SetUp { function testSettleNOPsOffchainSuccess() public { // deploy and configure a registry with OFF_CHAIN payout (Registry registry, ) = deployAndConfigureRegistryAndRegistrar(AutoBase.PayoutMode.OFF_CHAIN); + registry.setPayees(PAYEES); uint256[] memory payments = new uint256[](TRANSMITTERS.length); for (uint256 i = 0; i < TRANSMITTERS.length; i++) { @@ -991,6 +993,7 @@ contract NOPsSettlement is SetUp { function testSettleNOPsOffchainSuccessWithERC20MultiSteps() public { // deploy and configure a registry with OFF_CHAIN payout (Registry registry, ) = deployAndConfigureRegistryAndRegistrar(AutoBase.PayoutMode.OFF_CHAIN); + registry.setPayees(PAYEES); // register an upkeep and add funds uint256 id = registry.registerUpkeep(address(TARGET1), 1000000, UPKEEP_ADMIN, 0, address(usdToken18), "", "", ""); @@ -1186,6 +1189,7 @@ contract NOPsSettlement is SetUp { function testSinglePerformAndNodesCanWithdrawOnchain() public { // deploy and configure a registry with OFF_CHAIN payout (Registry registry, ) = deployAndConfigureRegistryAndRegistrar(AutoBase.PayoutMode.OFF_CHAIN); + registry.setPayees(PAYEES); // register an upkeep and add funds uint256 id = registry.registerUpkeep(address(TARGET1), 1000000, UPKEEP_ADMIN, 0, address(usdToken18), "", "", ""); @@ -1224,6 +1228,7 @@ contract NOPsSettlement is SetUp { function testMultiplePerformsAndNodesCanWithdrawOnchain() public { // deploy and configure a registry with OFF_CHAIN payout (Registry registry, ) = deployAndConfigureRegistryAndRegistrar(AutoBase.PayoutMode.OFF_CHAIN); + registry.setPayees(PAYEES); // register an upkeep and add funds uint256 id = registry.registerUpkeep(address(TARGET1), 1000000, UPKEEP_ADMIN, 0, address(usdToken18), "", "", ""); @@ -2604,6 +2609,7 @@ contract TransferPayeeship is SetUp { } function test_RevertsWhen_TransferToSelf() external { + registry.setPayees(PAYEES); vm.startPrank(PAYEES[0]); vm.expectRevert(Registry.ValueNotChanged.selector); @@ -2611,6 +2617,8 @@ contract TransferPayeeship is SetUp { } function test_Transfer_DoesNotChangePayee() external { + registry.setPayees(PAYEES); + vm.startPrank(PAYEES[0]); registry.transferPayeeship(TRANSMITTERS[0], randomAddress()); @@ -2620,6 +2628,8 @@ contract TransferPayeeship is SetUp { } function test_EmitEvent_CalledByPayee() external { + registry.setPayees(PAYEES); + vm.startPrank(PAYEES[0]); address newPayee = randomAddress(); @@ -2639,6 +2649,8 @@ contract AcceptPayeeship is SetUp { event PayeeshipTransferred(address indexed transmitter, address indexed from, address indexed to); function test_RevertsWhen_NotCalledByProposedPayee() external { + registry.setPayees(PAYEES); + vm.startPrank(PAYEES[0]); address newPayee = randomAddress(); registry.transferPayeeship(TRANSMITTERS[0], newPayee); @@ -2649,6 +2661,8 @@ contract AcceptPayeeship is SetUp { } function test_PayeeChanged() external { + registry.setPayees(PAYEES); + vm.startPrank(PAYEES[0]); address newPayee = randomAddress(); registry.transferPayeeship(TRANSMITTERS[0], newPayee); diff --git a/contracts/src/v0.8/automation/test/v2_3/BaseTest.t.sol b/contracts/src/v0.8/automation/test/v2_3/BaseTest.t.sol index b966a5dee06..95dcea3bdf4 100644 --- a/contracts/src/v0.8/automation/test/v2_3/BaseTest.t.sol +++ b/contracts/src/v0.8/automation/test/v2_3/BaseTest.t.sol @@ -283,7 +283,6 @@ contract BaseTest is Test { billingTokenAddresses, billingTokenConfigs ); - registry.setPayees(PAYEES); return (registry, registrar); } From 8bc27dc9f33e69d8a78ecf3e01d2ad75ff842d08 Mon Sep 17 00:00:00 2001 From: FelixFan1992 Date: Fri, 2 Aug 2024 11:12:24 -0400 Subject: [PATCH 11/13] update --- .../test/v2_3/AutomationRegistry2_3.t.sol | 37 ++++++++ .../automation/AutomationRegistry2_3.test.ts | 92 ------------------- 2 files changed, 37 insertions(+), 92 deletions(-) diff --git a/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol b/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol index bb1f2237d07..1369b221a6e 100644 --- a/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol +++ b/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol @@ -2678,6 +2678,8 @@ contract AcceptPayeeship is SetUp { } contract SetPayees is SetUp { + event PayeesUpdated(address[] transmitters, address[] payees); + function test_RevertsWhen_NotCalledByOwner() external { vm.startPrank(STRANGER); @@ -2700,4 +2702,39 @@ contract SetPayees is SetUp { vm.expectRevert(Registry.InvalidPayee.selector); registry.setPayees(NEW_PAYEES); } + + function test_SetPayees_WhenExistingPayeesAreEmpty() external { + (registry, ) = deployAndConfigureRegistryAndRegistrar(AutoBase.PayoutMode.ON_CHAIN); + + for (uint256 i = 0; i < TRANSMITTERS.length; i++) { + (, , , , address payee) = registry.getTransmitterInfo(TRANSMITTERS[i]); + assertEq(address(0), payee); + } + + vm.startPrank(registry.owner()); + + vm.expectEmit(); + emit PayeesUpdated(TRANSMITTERS, PAYEES); + registry.setPayees(PAYEES); + for (uint256 i = 0; i < TRANSMITTERS.length; i++) { + (bool active, , , , address payee) = registry.getTransmitterInfo(TRANSMITTERS[i]); + assertTrue(active); + assertEq(PAYEES[i], payee); + } + } + + function test_DotNotSetPayeesToIgnoredAddress() external { + address IGNORE_ADDRESS = 0xFFfFfFffFFfffFFfFFfFFFFFffFFFffffFfFFFfF; + (registry, ) = deployAndConfigureRegistryAndRegistrar(AutoBase.PayoutMode.ON_CHAIN); + PAYEES[0] = IGNORE_ADDRESS; + + registry.setPayees(PAYEES); + (bool active, , , , address payee) = registry.getTransmitterInfo(TRANSMITTERS[0]); + assertTrue(active); + assertEq(address(0), payee); + + (active, , , , payee) = registry.getTransmitterInfo(TRANSMITTERS[1]); + assertTrue(active); + assertEq(PAYEES[1], payee); + } } diff --git a/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts b/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts index a7acde283fc..2c1733b55b8 100644 --- a/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts +++ b/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts @@ -4299,98 +4299,6 @@ describe('AutomationRegistry2_3', () => { }) }) - describe('#setPayees', () => { - const IGNORE_ADDRESS = '0xFFfFfFffFFfffFFfFFfFFFFFffFFFffffFfFFFfF' - - itMaybe( - 'sets the payees when existing payees are zero address', - async () => { - //Initial payees should be zero address - await blankRegistry.connect(owner).setConfigTypeSafe(...baseConfig) // used to test initial config - - for (let i = 0; i < keeperAddresses.length; i++) { - const payee = ( - await blankRegistry.getTransmitterInfo(keeperAddresses[i]) - ).payee // used to test initial config - assert.equal(payee, zeroAddress) - } - - await blankRegistry.connect(owner).setPayees(payees) // used to test initial config - - for (let i = 0; i < keeperAddresses.length; i++) { - const payee = ( - await blankRegistry.getTransmitterInfo(keeperAddresses[i]) - ).payee - assert.equal(payee, payees[i]) - } - }, - ) - - it('does not change the payee if IGNORE_ADDRESS is used as payee', async () => { - const signers = Array.from({ length: 5 }, randomAddress) - const keepers = Array.from({ length: 5 }, randomAddress) - const payees = Array.from({ length: 5 }, randomAddress) - const newTransmitter = randomAddress() - const newPayee = randomAddress() - const ignoreAddresses = new Array(payees.length).fill(IGNORE_ADDRESS) - const newPayees = [...ignoreAddresses, newPayee] - // arbitrum registry - // configure registry with 5 keepers // optimism registry - await blankRegistry // used to test initial configurations - .connect(owner) - .setConfigTypeSafe( - signers, - keepers, - f, - config, - offchainVersion, - offchainBytes, - [], - [], - ) - // arbitrum registry - // set initial payees // optimism registry - await blankRegistry.connect(owner).setPayees(payees) // used to test initial configurations - // arbitrum registry - // add another keeper // optimism registry - await blankRegistry // used to test initial configurations - .connect(owner) - .setConfigTypeSafe( - [...signers, randomAddress()], - [...keepers, newTransmitter], - f, - config, - offchainVersion, - offchainBytes, - [], - [], - ) - // arbitrum registry - // update payee list // optimism registry // arbitrum registry - await blankRegistry.connect(owner).setPayees(newPayees) // used to test initial configurations // optimism registry - const ignored = await blankRegistry.getTransmitterInfo(newTransmitter) // used to test initial configurations - assert.equal(newPayee, ignored.payee) - assert.equal(ignored.active, true) - }) - - it('reverts if payee is non zero and owner tries to change payee', async () => { - const newPayees = [randomAddress(), ...payees.slice(1)] - - await evmRevertCustomError( - registry.connect(owner).setPayees(newPayees), - registry, - 'InvalidPayee', - ) - }) - - it('emits events for every payee added and removed', async () => { - const tx = await registry.connect(owner).setPayees(payees) - await expect(tx) - .to.emit(registry, 'PayeesUpdated') - .withArgs(keeperAddresses, payees) - }) - }) - describe('#cancelUpkeep', () => { describe('when called by the owner', async () => { it('immediately prevents upkeep', async () => { From 49c89265be3744be039aed8affc4a558f9082519 Mon Sep 17 00:00:00 2001 From: FelixFan1992 Date: Fri, 2 Aug 2024 16:37:57 -0400 Subject: [PATCH 12/13] withdraw --- .../test/v2_3/AutomationRegistry2_3.t.sol | 30 ++++ .../automation/AutomationRegistry2_3.test.ts | 153 ------------------ 2 files changed, 30 insertions(+), 153 deletions(-) diff --git a/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol b/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol index 1369b221a6e..fb26cbbe70c 100644 --- a/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol +++ b/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol @@ -2738,3 +2738,33 @@ contract SetPayees is SetUp { assertEq(PAYEES[1], payee); } } + +contract GetActiveUpkeepIDs is SetUp { + function test_RevertsWhen_IndexOutOfRange() external { + vm.expectRevert(Registry.IndexOutOfRange.selector); + registry.getActiveUpkeepIDs(5, 0); + + vm.expectRevert(Registry.IndexOutOfRange.selector); + registry.getActiveUpkeepIDs(6, 0); + } + + function test_ReturnsAllUpkeeps_WhenMaxCountIsZero() external { + uint256[] memory uids = registry.getActiveUpkeepIDs(0, 0); + assertEq(5, uids.length); + + uids = registry.getActiveUpkeepIDs(2, 0); + assertEq(3, uids.length); + } + + function test_ReturnsAllRemainingUpkeeps_WhenMaxCountIsTooLarge() external { + uint256[] memory uids = registry.getActiveUpkeepIDs(2, 20); + assertEq(3, uids.length); + } + + function test_ReturnsUpkeeps_BoundByMaxCount() external { + uint256[] memory uids = registry.getActiveUpkeepIDs(1, 2); + assertEq(2, uids.length); + assertEq(uids[0], linkUpkeepID2); + assertEq(uids[1], usdUpkeepID18); + } +} diff --git a/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts b/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts index 2c1733b55b8..3f28a4410b1 100644 --- a/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts +++ b/contracts/test/v0.8/automation/AutomationRegistry2_3.test.ts @@ -3106,44 +3106,6 @@ describe('AutomationRegistry2_3', () => { await getTransmitTx(registry, keeper1, [upkeepId2]) }) - it('reverts if called on a non existing ID', async () => { - await evmRevertCustomError( - registry - .connect(admin) - .withdrawFunds(upkeepId.add(1), await payee1.getAddress()), - registry, - 'OnlyCallableByAdmin', - ) - }) - - it('reverts if called by anyone but the admin', async () => { - await evmRevertCustomError( - registry - .connect(owner) - .withdrawFunds(upkeepId, await payee1.getAddress()), - registry, - 'OnlyCallableByAdmin', - ) - }) - - it('reverts if called on an uncanceled upkeep', async () => { - await evmRevertCustomError( - registry - .connect(admin) - .withdrawFunds(upkeepId, await payee1.getAddress()), - registry, - 'UpkeepNotCanceled', - ) - }) - - it('reverts if called with the 0 address', async () => { - await evmRevertCustomError( - registry.connect(admin).withdrawFunds(upkeepId, zeroAddress), - registry, - 'InvalidRecipient', - ) - }) - describe('after the registration is paused, then cancelled', () => { it('allows the admin to withdraw', async () => { const balance = await registry.getBalance(upkeepId) @@ -3513,46 +3475,6 @@ describe('AutomationRegistry2_3', () => { }) }) - describe('#getActiveUpkeepIDs', () => { - it('reverts if startIndex is out of bounds ', async () => { - await evmRevertCustomError( - registry.getActiveUpkeepIDs(numUpkeeps, 0), - registry, - 'IndexOutOfRange', - ) - await evmRevertCustomError( - registry.getActiveUpkeepIDs(numUpkeeps + 1, 0), - registry, - 'IndexOutOfRange', - ) - }) - - it('returns upkeep IDs bounded by maxCount', async () => { - let upkeepIds = await registry.getActiveUpkeepIDs(0, 1) - assert(upkeepIds.length == 1) - assert(upkeepIds[0].eq(upkeepId)) - upkeepIds = await registry.getActiveUpkeepIDs(1, 3) - assert(upkeepIds.length == 3) - expect(upkeepIds).to.deep.equal([ - afUpkeepId, - logUpkeepId, - streamsLookupUpkeepId, - ]) - }) - - it('returns as many ids as possible if maxCount > num available', async () => { - const upkeepIds = await registry.getActiveUpkeepIDs(1, numUpkeeps + 100) - assert(upkeepIds.length == numUpkeeps - 1) - }) - - it('returns all upkeep IDs if maxCount is 0', async () => { - let upkeepIds = await registry.getActiveUpkeepIDs(0, 0) - assert(upkeepIds.length == numUpkeeps) - upkeepIds = await registry.getActiveUpkeepIDs(2, 0) - assert(upkeepIds.length == numUpkeeps - 2) - }) - }) - describe('#getMaxPaymentForGas', () => { let maxl1CostWeiArbWithoutMultiplier: BigNumber let maxl1CostWeiOptWithoutMultiplier: BigNumber @@ -4224,81 +4146,6 @@ describe('AutomationRegistry2_3', () => { }) }) - describe('#withdrawOwnerFunds', () => { - it('can only be called by finance admin', async () => { - await evmRevertCustomError( - registry.connect(keeper1).withdrawLink(zeroAddress, 1), - registry, - 'OnlyFinanceAdmin', - ) - }) - - itMaybe('withdraws the collected fees to owner', async () => { - await registry.connect(admin).addFunds(upkeepId, toWei('100')) - const financeAdminAddress = await financeAdmin.getAddress() - // Very high min spend, whole balance as cancellation fees - const newMinUpkeepSpend = toWei('1000') - await registry.connect(owner).setConfigTypeSafe( - signerAddresses, - keeperAddresses, - f, - { - checkGasLimit, - stalenessSeconds, - gasCeilingMultiplier, - maxCheckDataSize, - maxPerformDataSize, - maxRevertDataSize, - maxPerformGas, - fallbackGasPrice, - fallbackLinkPrice, - fallbackNativePrice, - transcoder: transcoder.address, - registrars: [], - upkeepPrivilegeManager: upkeepManager, - chainModule: chainModuleBase.address, - reorgProtectionEnabled: true, - financeAdmin: financeAdminAddress, - }, - offchainVersion, - offchainBytes, - [linkToken.address], - [ - { - gasFeePPB: paymentPremiumPPB, - flatFeeMilliCents, - priceFeed: linkUSDFeed.address, - fallbackPrice: fallbackLinkPrice, - minSpend: newMinUpkeepSpend, - decimals: 18, - }, - ], - ) - const upkeepBalance = (await registry.getUpkeep(upkeepId)).balance - const ownerBefore = await linkToken.balanceOf(await owner.getAddress()) - - await registry.connect(owner).cancelUpkeep(upkeepId) - - // Transfered to owner balance on registry - let ownerRegistryBalance = await registry.linkAvailableForPayment() - assert.isTrue(ownerRegistryBalance.eq(upkeepBalance)) - - // Now withdraw - await registry - .connect(financeAdmin) - .withdrawLink(await owner.getAddress(), ownerRegistryBalance) - - ownerRegistryBalance = await registry.linkAvailableForPayment() - const ownerAfter = await linkToken.balanceOf(await owner.getAddress()) - - // Owner registry balance should be changed to 0 - assert.isTrue(ownerRegistryBalance.eq(BigNumber.from('0'))) - - // Owner should be credited with the balance - assert.isTrue(ownerBefore.add(upkeepBalance).eq(ownerAfter)) - }) - }) - describe('#cancelUpkeep', () => { describe('when called by the owner', async () => { it('immediately prevents upkeep', async () => { From 4005c8c2c70a21e92d30e97df282ded7df2322a5 Mon Sep 17 00:00:00 2001 From: FelixFan1992 Date: Wed, 7 Aug 2024 17:40:35 -0400 Subject: [PATCH 13/13] update --- .../test/v2_3/AutomationRegistry2_3.t.sol | 43 ++++++------------- .../v0.8/automation/test/v2_3/BaseTest.t.sol | 20 +++++++-- 2 files changed, 30 insertions(+), 33 deletions(-) diff --git a/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol b/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol index fb26cbbe70c..41aabf1bbe2 100644 --- a/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol +++ b/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol @@ -349,7 +349,7 @@ contract Withdraw is SetUp { // default is ON_CHAIN mode function test_WithdrawERC20Fees_RevertsWhen_LinkAvailableForPaymentIsNegative() public { - _transmit(usdUpkeepID18, registry, bytes4(0)); // adds USD token to finance withdrawable, and gives NOPs a LINK balance + _transmit(usdUpkeepID18, registry); // adds USD token to finance withdrawable, and gives NOPs a LINK balance require(registry.linkAvailableForPayment() < 0, "linkAvailableForPayment should be negative"); require( registry.getAvailableERC20ForPayment(address(usdToken18)) > 0, @@ -375,7 +375,7 @@ contract Withdraw is SetUp { registry.addFunds(id, 1e20); // manually create a transmit so transmitters earn some rewards - _transmit(id, registry, bytes4(0)); + _transmit(id, registry); require(registry.linkAvailableForPayment() < 0, "linkAvailableForPayment should be negative"); vm.prank(FINANCE_ADMIN); registry.withdrawERC20Fees(address(usdToken18), aMockAddress, 1); // finance can withdraw @@ -1003,7 +1003,7 @@ contract NOPsSettlement is SetUp { registry.addFunds(id, 1e20); // manually create a transmit so transmitters earn some rewards - _transmit(id, registry, bytes4(0)); + _transmit(id, registry); // verify transmitters have positive balances uint256[] memory payments = new uint256[](TRANSMITTERS.length); @@ -1040,7 +1040,7 @@ contract NOPsSettlement is SetUp { vm.startPrank(UPKEEP_ADMIN); vm.roll(100 + block.number); // manually create a transmit so transmitters earn some rewards - _transmit(id, registry, bytes4(0)); + _transmit(id, registry); uint256 erc20ForPayment2 = registry.getAvailableERC20ForPayment(address(usdToken18)); require(erc20ForPayment2 > erc20ForPayment1, "ERC20AvailableForPayment should be greater after another transmit"); @@ -1083,13 +1083,13 @@ contract NOPsSettlement is SetUp { registry.addFunds(id, 1e20); // manually create a transmit so TRANSMITTERS earn some rewards - _transmit(id, registry, bytes4(0)); + _transmit(id, registry); // TRANSMITTERS have positive balance now // configure the registry to use NEW_TRANSMITTERS _configureWithNewTransmitters(registry, registrar); - _transmit(id, registry, bytes4(0)); + _transmit(id, registry); // verify all transmitters have positive balances address[] memory expectedPayees = new address[](6); @@ -1199,7 +1199,7 @@ contract NOPsSettlement is SetUp { registry.addFunds(id, 1e20); // manually create a transmit so transmitters earn some rewards - _transmit(id, registry, bytes4(0)); + _transmit(id, registry); // disable offchain payments _mintLink(address(registry), 1e19); @@ -1240,7 +1240,7 @@ contract NOPsSettlement is SetUp { // manually call transmit so transmitters earn some rewards for (uint256 i = 0; i < 50; i++) { vm.roll(100 + block.number); - _transmit(id, registry, bytes4(0)); + _transmit(id, registry); } // disable offchain payments @@ -1251,7 +1251,7 @@ contract NOPsSettlement is SetUp { // manually call transmit after offchain payment is disabled for (uint256 i = 0; i < 50; i++) { vm.roll(100 + block.number); - _transmit(id, registry, bytes4(0)); + _transmit(id, registry); } // payees should be able to withdraw onchain @@ -1676,7 +1676,7 @@ contract Transmit is SetUp { require(registry.getAvailableERC20ForPayment(address(weth)) == 0, "ERC20AvailableForPayment should be 0"); // do the thing - _transmit(upkeepIDs, registry, bytes4(0)); + _transmit(upkeepIDs, registry); // withdraw-able by the finance team should be positive require( @@ -1731,7 +1731,7 @@ contract Transmit is SetUp { // manually create a transmit vm.recordLogs(); - _transmit(upkeepID, registry, bytes4(0)); + _transmit(upkeepID, registry); Vm.Log[] memory entries = vm.getRecordedLogs(); assertEq(entries.length, 3); @@ -1782,7 +1782,7 @@ contract Transmit is SetUp { // manually create a transmit vm.recordLogs(); - _transmit(upkeepID, registry, bytes4(0)); + _transmit(upkeepID, registry); Vm.Log[] memory entries = vm.getRecordedLogs(); assertEq(entries.length, 3); @@ -1998,28 +1998,11 @@ contract Pause is SetUp { assertTrue(state.paused); } - function test_revertsWhen_registerUpkeepInPausedRegistry() external { - vm.startPrank(registry.owner()); - registry.pause(); - - vm.expectRevert(Registry.RegistryPaused.selector); - uint256 id = registry.registerUpkeep( - address(TARGET1), - config.maxPerformGas, - UPKEEP_ADMIN, - uint8(Trigger.CONDITION), - address(linkToken), - "", - "", - "" - ); - } - function test_revertsWhen_transmitInPausedRegistry() external { vm.startPrank(registry.owner()); registry.pause(); - _transmit(usdUpkeepID18, registry, Registry.RegistryPaused.selector); + _transmitAndExpectRevert(usdUpkeepID18, registry, Registry.RegistryPaused.selector); } } diff --git a/contracts/src/v0.8/automation/test/v2_3/BaseTest.t.sol b/contracts/src/v0.8/automation/test/v2_3/BaseTest.t.sol index 95dcea3bdf4..07ef0a06e3d 100644 --- a/contracts/src/v0.8/automation/test/v2_3/BaseTest.t.sol +++ b/contracts/src/v0.8/automation/test/v2_3/BaseTest.t.sol @@ -355,13 +355,27 @@ contract BaseTest is Test { ); } - function _transmit(uint256 id, Registry registry, bytes4 selector) internal { + // tests single upkeep, expects success + function _transmit(uint256 id, Registry registry) internal { uint256[] memory ids = new uint256[](1); ids[0] = id; - _transmit(ids, registry, selector); + _handleTransmit(ids, registry, bytes4(0)); } - function _transmit(uint256[] memory ids, Registry registry, bytes4 selector) internal { + // tests multiple upkeeps, expects success + function _transmit(uint256[] memory ids, Registry registry) internal { + _handleTransmit(ids, registry, bytes4(0)); + } + + // tests single upkeep, expects revert + function _transmitAndExpectRevert(uint256 id, Registry registry, bytes4 selector) internal { + uint256[] memory ids = new uint256[](1); + ids[0] = id; + _handleTransmit(ids, registry, selector); + } + + // private function not exposed to actual testing contract + function _handleTransmit(uint256[] memory ids, Registry registry, bytes4 selector) private { bytes memory reportBytes; { uint256[] memory upkeepIds = new uint256[](ids.length);