Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add reorgProtectionEnabled feature flag in registry 2.2 #11862

Merged
merged 18 commits into from
Jan 25, 2024
Merged
12 changes: 4 additions & 8 deletions contracts/scripts/generate-automation-master-interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
* @description this script generates a master interface for interacting with the automation registry
* @notice run this script with pnpm ts-node ./scripts/generate-automation-master-interface.ts
*/
import { KeeperRegistry2_2__factory as KeeperRegistry } from '../typechain/factories/KeeperRegistry2_2__factory'
import { KeeperRegistryLogicA2_2__factory as KeeperRegistryLogicA } from '../typechain/factories/KeeperRegistryLogicA2_2__factory'
import { KeeperRegistryLogicB2_2__factory as KeeperRegistryLogicB } from '../typechain/factories/KeeperRegistryLogicB2_2__factory'
import { AutomationRegistry2_2__factory as Registry } from '../typechain/factories/AutomationRegistry2_2__factory'
import { AutomationRegistryLogicA2_2__factory as RegistryLogicA } from '../typechain/factories/AutomationRegistryLogicA2_2__factory'
import { AutomationRegistryLogicB2_2__factory as RegistryLogicB } from '../typechain/factories/AutomationRegistryLogicB2_2__factory'
import { utils } from 'ethers'
import fs from 'fs'
import { exec } from 'child_process'
Expand All @@ -15,11 +15,7 @@ const tmpDest = `${dest}/tmp.txt`

const combinedABI = []
const abiSet = new Set()
const abis = [
KeeperRegistry.abi,
KeeperRegistryLogicA.abi,
KeeperRegistryLogicB.abi,
]
const abis = [Registry.abi, RegistryLogicA.abi, RegistryLogicB.abi]

for (const abi of abis) {
for (const entry of abi) {
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,7 @@ contract AutomationRegistry2_2 is AutomationRegistryBase2_2, OCR2Abstract, Chain
});
s_fallbackGasPrice = onchainConfig.fallbackGasPrice;
s_fallbackLinkPrice = onchainConfig.fallbackLinkPrice;
skipReorgProtection = onchainConfig.skipReorgProtection;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

n00b: why prefix s_? with prefix s_, s_fallbackLinkPrice is a state variable (persistent storage onchain)? skipReorgProtection is just a local variable which will not be recorded onchain?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s_ means this is a storage variable
i_ means this is a immutable variable


uint32 previousConfigBlockNumber = s_storage.latestConfigBlockNumber;
s_storage.latestConfigBlockNumber = uint32(_blockNum());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ abstract contract AutomationRegistryBase2_2 is ConfirmedOwner, ExecutionPreventi
AggregatorV3Interface internal immutable i_fastGasFeed;
Mode internal immutable i_mode;
address internal immutable i_automationForwarderLogic;
bool internal skipReorgProtection;
FelixFan1992 marked this conversation as resolved.
Show resolved Hide resolved

/**
* @dev - The storage is gas optimised for one and only one function - transmit. All the storage accessed in transmit
Expand Down Expand Up @@ -208,6 +209,7 @@ abstract contract AutomationRegistryBase2_2 is ConfirmedOwner, ExecutionPreventi
* @member transcoder address of the transcoder contract
* @member registrars addresses of the registrar contracts
* @member upkeepPrivilegeManager address which can set privilege for upkeeps
* @member skipReorgProtection if this registry will skip re-org protection checks
*/
struct OnchainConfig {
uint32 paymentPremiumPPB;
Expand All @@ -225,6 +227,7 @@ abstract contract AutomationRegistryBase2_2 is ConfirmedOwner, ExecutionPreventi
address transcoder;
address[] registrars;
address upkeepPrivilegeManager;
bool skipReorgProtection;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we struct pack this a little better? Ex there is a free slot after minUpkeepSpend - also, some comments on where the words begin & end would be nice, like what we do for the Storage struct

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done. restructure this using Spack

}

/**
Expand Down Expand Up @@ -740,7 +743,7 @@ abstract contract AutomationRegistryBase2_2 is ConfirmedOwner, ExecutionPreventi
if (!_validateConditionalTrigger(upkeepId, rawTrigger, transmitInfo)) return (false, dedupID);
} else if (transmitInfo.triggerType == Trigger.LOG) {
bool valid;
(valid, dedupID) = _validateLogTrigger(upkeepId, rawTrigger, transmitInfo);
(valid, dedupID) = _validateLogTrigger(upkeepId, rawTrigger);
if (!valid) return (false, dedupID);
} else {
revert InvalidTriggerType();
Expand Down Expand Up @@ -774,7 +777,8 @@ abstract contract AutomationRegistryBase2_2 is ConfirmedOwner, ExecutionPreventi
return false;
}
if (
(trigger.blockHash != bytes32("") && _blockHash(trigger.blockNum) != trigger.blockHash) ||
(!skipReorgProtection &&
(trigger.blockHash != bytes32("") && _blockHash(trigger.blockNum) != trigger.blockHash)) ||
FelixFan1992 marked this conversation as resolved.
Show resolved Hide resolved
trigger.blockNum >= _blockNum()
) {
// There are two cases of reorged report
Expand All @@ -789,15 +793,12 @@ abstract contract AutomationRegistryBase2_2 is ConfirmedOwner, ExecutionPreventi
return true;
}

function _validateLogTrigger(
uint256 upkeepId,
bytes memory rawTrigger,
UpkeepTransmitInfo memory transmitInfo
FelixFan1992 marked this conversation as resolved.
Show resolved Hide resolved
) internal returns (bool, bytes32) {
function _validateLogTrigger(uint256 upkeepId, bytes memory rawTrigger) internal returns (bool, bytes32) {
LogTrigger memory trigger = abi.decode(rawTrigger, (LogTrigger));
bytes32 dedupID = keccak256(abi.encodePacked(upkeepId, trigger.logBlockHash, trigger.txHash, trigger.logIndex));
if (
(trigger.blockHash != bytes32("") && _blockHash(trigger.blockNum) != trigger.blockHash) ||
(!skipReorgProtection &&
(trigger.blockHash != bytes32("") && _blockHash(trigger.blockNum) != trigger.blockHash)) ||
FelixFan1992 marked this conversation as resolved.
Show resolved Hide resolved
trigger.blockNum >= _blockNum()
) {
// Reorg protection is same as conditional trigger upkeeps
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,8 @@ contract AutomationRegistryLogicB2_2 is AutomationRegistryBase2_2 {
fallbackLinkPrice: s_fallbackLinkPrice,
transcoder: s_storage.transcoder,
registrars: s_registrars.values(),
upkeepPrivilegeManager: s_storage.upkeepPrivilegeManager
upkeepPrivilegeManager: s_storage.upkeepPrivilegeManager,
skipReorgProtection: skipReorgProtection
});

return (state, config, s_signersList, s_transmittersList, s_hotVars.f);
Expand Down
47 changes: 27 additions & 20 deletions contracts/test/v0.8/automation/AutomationRegistry2_2.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,17 +34,17 @@ import { UpkeepTranscoder } from '../../../typechain/UpkeepTranscoder'
import { UpkeepAutoFunder } from '../../../typechain'
import {
CancelledUpkeepReportEvent,
IKeeperRegistryMaster as IKeeperRegistry,
IAutomationRegistryMaster as IAutomationRegistry,
InsufficientFundsUpkeepReportEvent,
ReorgedUpkeepReportEvent,
StaleUpkeepReportEvent,
UpkeepPerformedEvent,
} from '../../../typechain/IKeeperRegistryMaster'
} from '../../../typechain/IAutomationRegistryMaster'
import {
deployMockContract,
MockContract,
} from '@ethereum-waffle/mock-contract'
import { deployRegistry21 } from './helpers'
import { deployRegistry22 } from './helpers'

const describeMaybe = process.env.SKIP_SLOW ? describe.skip : describe
const itMaybe = process.env.SKIP_SLOW ? it.skip : it
Expand All @@ -68,9 +68,10 @@ enum Mode {
DEFAULT,
ARBITRUM,
OPTIMISM,
SCROLL,
FelixFan1992 marked this conversation as resolved.
Show resolved Hide resolved
}

// copied from KeeperRegistryBase2_2.sol
// copied from AutomationRegistryBase2_2.sol
enum Trigger {
CONDITION,
LOG,
Expand Down Expand Up @@ -149,11 +150,11 @@ let personas: Personas
let linkToken: Contract
let linkEthFeed: MockV3Aggregator
let gasPriceFeed: MockV3Aggregator
let registry: IKeeperRegistry // default registry, used for most tests
let arbRegistry: IKeeperRegistry // arbitrum registry
let opRegistry: IKeeperRegistry // optimism registry
let mgRegistry: IKeeperRegistry // "migrate registry" used in migration tests
let blankRegistry: IKeeperRegistry // used to test initial configurations
let registry: IAutomationRegistry // default registry, used for most tests
let arbRegistry: IAutomationRegistry // arbitrum registry
let opRegistry: IAutomationRegistry // optimism registry
let mgRegistry: IAutomationRegistry // "migrate registry" used in migration tests
let blankRegistry: IAutomationRegistry // used to test initial configurations
let mock: UpkeepMock
let autoFunderUpkeep: UpkeepAutoFunder
let ltUpkeep: MockContract
Expand Down Expand Up @@ -388,7 +389,7 @@ const parseCancelledUpkeepReportLogs = (receipt: ContractReceipt) => {
return parsedLogs
}

describe('KeeperRegistry2_2', () => {
describe('AutomationRegistry2_2', () => {
let owner: Signer
let keeper1: Signer
let keeper2: Signer
Expand Down Expand Up @@ -418,7 +419,7 @@ describe('KeeperRegistry2_2', () => {
let signers: Wallet[]
let signerAddresses: string[]
let config: any
let baseConfig: Parameters<IKeeperRegistry['setConfig']>
let baseConfig: Parameters<IAutomationRegistry['setConfig']>
let upkeepManager: string

before(async () => {
Expand Down Expand Up @@ -564,7 +565,7 @@ describe('KeeperRegistry2_2', () => {
}

const verifyMaxPayment = async (
registry: IKeeperRegistry,
registry: IAutomationRegistry,
l1CostWei?: BigNumber,
) => {
type TestCase = {
Expand Down Expand Up @@ -628,6 +629,7 @@ describe('KeeperRegistry2_2', () => {
transcoder: transcoder.address,
registrars: [],
upkeepPrivilegeManager: upkeepManager,
skipReorgProtection: false,
}),
offchainVersion,
offchainBytes,
Expand Down Expand Up @@ -708,7 +710,7 @@ describe('KeeperRegistry2_2', () => {
}

const getTransmitTx = async (
registry: IKeeperRegistry,
registry: IAutomationRegistry,
transmitter: Signer,
upkeepIds: BigNumber[],
overrides: GetTransmitTXOptions = {},
Expand Down Expand Up @@ -794,7 +796,7 @@ describe('KeeperRegistry2_2', () => {
}

const getTransmitTxWithReport = async (
registry: IKeeperRegistry,
registry: IAutomationRegistry,
transmitter: Signer,
report: BytesLike,
) => {
Expand Down Expand Up @@ -891,39 +893,39 @@ describe('KeeperRegistry2_2', () => {
offchainBytes,
]

registry = await deployRegistry21(
registry = await deployRegistry22(
owner,
Mode.DEFAULT,
linkToken.address,
linkEthFeed.address,
gasPriceFeed.address,
)

arbRegistry = await deployRegistry21(
arbRegistry = await deployRegistry22(
owner,
Mode.ARBITRUM,
linkToken.address,
linkEthFeed.address,
gasPriceFeed.address,
)

opRegistry = await deployRegistry21(
opRegistry = await deployRegistry22(
owner,
Mode.OPTIMISM,
linkToken.address,
linkEthFeed.address,
gasPriceFeed.address,
)

mgRegistry = await deployRegistry21(
mgRegistry = await deployRegistry22(
owner,
Mode.DEFAULT,
linkToken.address,
linkEthFeed.address,
gasPriceFeed.address,
)

blankRegistry = await deployRegistry21(
blankRegistry = await deployRegistry22(
owner,
Mode.DEFAULT,
linkToken.address,
Expand Down Expand Up @@ -3453,7 +3455,7 @@ describe('KeeperRegistry2_2', () => {
describe('#typeAndVersion', () => {
it('uses the correct type and version', async () => {
const typeAndVersion = await registry.typeAndVersion()
assert.equal(typeAndVersion, 'KeeperRegistry 2.1.0')
assert.equal(typeAndVersion, 'AutomationRegistry 2.2.0')
})
})

Expand Down Expand Up @@ -3544,6 +3546,7 @@ describe('KeeperRegistry2_2', () => {
transcoder: newTranscoder,
registrars: newRegistrars,
upkeepPrivilegeManager: upkeepManager,
skipReorgProtection: false,
}

it('reverts when called by anyone but the proposed owner', async () => {
Expand Down Expand Up @@ -4565,6 +4568,7 @@ describe('KeeperRegistry2_2', () => {
transcoder: transcoder.address,
registrars: [],
upkeepPrivilegeManager: upkeepManager,
skipReorgProtection: false,
},
offchainVersion,
offchainBytes,
Expand Down Expand Up @@ -5209,6 +5213,7 @@ describe('KeeperRegistry2_2', () => {
transcoder: transcoder.address,
registrars: [],
upkeepPrivilegeManager: upkeepManager,
skipReorgProtection: false,
},
offchainVersion,
offchainBytes,
Expand Down Expand Up @@ -5262,6 +5267,7 @@ describe('KeeperRegistry2_2', () => {
transcoder: transcoder.address,
registrars: [],
upkeepPrivilegeManager: upkeepManager,
skipReorgProtection: false,
},
offchainVersion,
offchainBytes,
Expand Down Expand Up @@ -5310,6 +5316,7 @@ describe('KeeperRegistry2_2', () => {
transcoder: transcoder.address,
registrars: [],
upkeepPrivilegeManager: upkeepManager,
skipReorgProtection: false,
},
offchainVersion,
offchainBytes,
Expand Down
31 changes: 31 additions & 0 deletions contracts/test/v0.8/automation/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ import { ethers } from 'hardhat'
import { KeeperRegistryLogicB2_1__factory as KeeperRegistryLogicBFactory } from '../../../typechain/factories/KeeperRegistryLogicB2_1__factory'
import { IKeeperRegistryMaster as IKeeperRegistry } from '../../../typechain/IKeeperRegistryMaster'
import { IKeeperRegistryMaster__factory as IKeeperRegistryMasterFactory } from '../../../typechain/factories/IKeeperRegistryMaster__factory'
import { AutomationRegistryLogicB2_2__factory as AutomationRegistryLogicBFactory } from '../../../typechain/factories/AutomationRegistryLogicB2_2__factory'
import { IAutomationRegistryMaster as IAutomationRegistry } from '../../../typechain/IAutomationRegistryMaster'
import { IAutomationRegistryMaster__factory as IAutomationRegistryMasterFactory } from '../../../typechain/factories/IAutomationRegistryMaster__factory'

export const deployRegistry21 = async (
from: Signer,
Expand All @@ -29,3 +32,31 @@ export const deployRegistry21 = async (
const master = await registryFactory.connect(from).deploy(logicA.address)
return IKeeperRegistryMasterFactory.connect(master.address, from)
}

export const deployRegistry22 = async (
from: Signer,
mode: Parameters<AutomationRegistryLogicBFactory['deploy']>[0],
link: Parameters<AutomationRegistryLogicBFactory['deploy']>[1],
linkNative: Parameters<AutomationRegistryLogicBFactory['deploy']>[2],
fastgas: Parameters<AutomationRegistryLogicBFactory['deploy']>[3],
): Promise<IAutomationRegistry> => {
const logicBFactory = await ethers.getContractFactory(
'AutomationRegistryLogicB2_2',
)
const logicAFactory = await ethers.getContractFactory(
'AutomationRegistryLogicA2_2',
)
const registryFactory = await ethers.getContractFactory(
'AutomationRegistry2_2',
)
const forwarderLogicFactory = await ethers.getContractFactory(
'AutomationForwarderLogic',
)
const forwarderLogic = await forwarderLogicFactory.connect(from).deploy()
const logicB = await logicBFactory
.connect(from)
.deploy(mode, link, linkNative, fastgas, forwarderLogic.address)
const logicA = await logicAFactory.connect(from).deploy(logicB.address)
const master = await registryFactory.connect(from).deploy(logicA.address)
return IAutomationRegistryMasterFactory.connect(master.address, from)
}
Loading