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 @@ -101,7 +101,8 @@ contract AutomationRegistry2_2 is AutomationRegistryBase2_2, OCR2Abstract, Chain
(upkeepTransmitInfo[i].earlyChecksPassed, upkeepTransmitInfo[i].dedupID) = _prePerformChecks(
report.upkeepIds[i],
report.triggers[i],
upkeepTransmitInfo[i]
upkeepTransmitInfo[i],
hotVars
);

if (upkeepTransmitInfo[i].earlyChecksPassed) {
Expand Down Expand Up @@ -308,7 +309,8 @@ contract AutomationRegistry2_2 is AutomationRegistryBase2_2, OCR2Abstract, Chain
paused: s_hotVars.paused,
reentrancyGuard: s_hotVars.reentrancyGuard,
totalPremium: totalPremium,
latestEpoch: 0 // DON restarts epoch
latestEpoch: 0, // DON restarts epoch
skipReorgProtection: onchainConfig.skipReorgProtection
});

s_storage = Storage({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,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 +226,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 @@ -307,16 +309,16 @@ abstract contract AutomationRegistryBase2_2 is ConfirmedOwner, ExecutionPreventi

/// @dev Config + State storage struct which is on hot transmit path
struct HotVars {
uint8 f; // maximum number of faulty oracles
uint32 paymentPremiumPPB; // premium percentage charged to user over tx cost
uint32 flatFeeMicroLink; // flat fee charged to user for every perform
uint24 stalenessSeconds; // Staleness tolerance for feeds
uint16 gasCeilingMultiplier; // multiplier on top of fast gas feed for upper bound
bool paused; // pause switch for all upkeeps in the registry
bool reentrancyGuard; // guard against reentrancy
uint96 totalPremium; // total historical payment to oracles for premium
uint32 latestEpoch; // latest epoch for which a report was transmitted
// 1 EVM word full
uint96 totalPremium; // ─────────╮ total historical payment to oracles for premium
uint32 paymentPremiumPPB; // premium percentage charged to user over tx cost
uint32 flatFeeMicroLink; // flat fee charged to user for every perform
uint32 latestEpoch; // │ latest epoch for which a report was transmitted
uint24 stalenessSeconds; // │ Staleness tolerance for feeds
uint16 gasCeilingMultiplier; // │ multiplier on top of fast gas feed for upper bound
uint8 f; // │ maximum number of faulty oracles
bool paused; // │ pause switch for all upkeeps in the registry
bool reentrancyGuard; // ────────╯ guard against reentrancy
bool skipReorgProtection; // if this registry should skip re-org protection mechanism
Copy link
Contributor

Choose a reason for hiding this comment

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

idea for the future - we can totally combine these booleans into a single uint8 using a bitmap, then the struct would be 1 word again :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I created a ticket and assigned to myself: https://smartcontract-it.atlassian.net/browse/AUTO-8859

}

/// @dev Config + State storage struct which is not on hot transmit path
Expand Down Expand Up @@ -733,14 +735,15 @@ abstract contract AutomationRegistryBase2_2 is ConfirmedOwner, ExecutionPreventi
function _prePerformChecks(
uint256 upkeepId,
bytes memory rawTrigger,
UpkeepTransmitInfo memory transmitInfo
UpkeepTransmitInfo memory transmitInfo,
HotVars memory hotVars
) internal returns (bool, bytes32) {
bytes32 dedupID;
if (transmitInfo.triggerType == Trigger.CONDITION) {
if (!_validateConditionalTrigger(upkeepId, rawTrigger, transmitInfo)) return (false, dedupID);
if (!_validateConditionalTrigger(upkeepId, rawTrigger, transmitInfo, hotVars)) return (false, dedupID);
} else if (transmitInfo.triggerType == Trigger.LOG) {
bool valid;
(valid, dedupID) = _validateLogTrigger(upkeepId, rawTrigger, transmitInfo);
(valid, dedupID) = _validateLogTrigger(upkeepId, rawTrigger, hotVars);
if (!valid) return (false, dedupID);
} else {
revert InvalidTriggerType();
Expand All @@ -765,7 +768,8 @@ abstract contract AutomationRegistryBase2_2 is ConfirmedOwner, ExecutionPreventi
function _validateConditionalTrigger(
uint256 upkeepId,
bytes memory rawTrigger,
UpkeepTransmitInfo memory transmitInfo
UpkeepTransmitInfo memory transmitInfo,
HotVars memory hotVars
) internal returns (bool) {
ConditionalTrigger memory trigger = abi.decode(rawTrigger, (ConditionalTrigger));
if (trigger.blockNum < transmitInfo.upkeep.lastPerformedBlockNumber) {
Expand All @@ -774,7 +778,8 @@ abstract contract AutomationRegistryBase2_2 is ConfirmedOwner, ExecutionPreventi
return false;
}
if (
(trigger.blockHash != bytes32("") && _blockHash(trigger.blockNum) != trigger.blockHash) ||
(!hotVars.skipReorgProtection &&
(trigger.blockHash != bytes32("") && _blockHash(trigger.blockNum) != trigger.blockHash)) ||
trigger.blockNum >= _blockNum()
) {
// There are two cases of reorged report
Expand All @@ -792,12 +797,13 @@ abstract contract AutomationRegistryBase2_2 is ConfirmedOwner, ExecutionPreventi
function _validateLogTrigger(
uint256 upkeepId,
bytes memory rawTrigger,
UpkeepTransmitInfo memory transmitInfo
FelixFan1992 marked this conversation as resolved.
Show resolved Hide resolved
HotVars memory hotVars
) 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) ||
(!hotVars.skipReorgProtection &&
FelixFan1992 marked this conversation as resolved.
Show resolved Hide resolved
(trigger.blockHash != bytes32("") && _blockHash(trigger.blockNum) != trigger.blockHash)) ||
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: s_hotVars.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