Skip to content

Commit

Permalink
replace trigger bytes with triggerID in logs (#9712)
Browse files Browse the repository at this point in the history
  • Loading branch information
@RyanRHall
RyanRHall authored Jul 10, 2023
1 parent ae49f97 commit 84ee2b5
Show file tree
Hide file tree
Showing 9 changed files with 146 additions and 131 deletions.
30 changes: 17 additions & 13 deletions contracts/src/v0.8/dev/automation/2_1/KeeperRegistry2_1.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,11 @@ contract KeeperRegistry2_1 is KeeperRegistryBase2_1, OCR2Abstract, Chainable, ER
report.linkNative,
true
);
upkeepTransmitInfo[i].triggerID = _triggerID(report.upkeepIds[i], report.triggers[i]);
upkeepTransmitInfo[i].earlyChecksPassed = _prePerformChecks(
report.upkeepIds[i],
upkeepTransmitInfo[i].triggerType,
upkeepTransmitInfo[i].triggerID,
report.triggers[i],
upkeepTransmitInfo[i].upkeep,
upkeepTransmitInfo[i].maxLinkPayment
Expand Down Expand Up @@ -167,7 +169,7 @@ contract KeeperRegistry2_1 is KeeperRegistryBase2_1, OCR2Abstract, Chainable, ER
reimbursement + premium,
upkeepTransmitInfo[i].gasUsed,
upkeepTransmitInfo[i].gasOverhead,
report.triggers[i]
upkeepTransmitInfo[i].triggerID
);
}
}
Expand Down Expand Up @@ -403,27 +405,28 @@ contract KeeperRegistry2_1 is KeeperRegistryBase2_1, OCR2Abstract, Chainable, ER
function _prePerformChecks(
uint256 upkeepId,
Trigger triggerType,
bytes32 triggerID,
bytes memory rawTrigger,
Upkeep memory upkeep,
uint96 maxLinkPayment
) internal returns (bool) {
if (triggerType == Trigger.CONDITION) {
if (!_validateConditionalTrigger(upkeepId, rawTrigger, upkeep)) return false;
if (!_validateConditionalTrigger(upkeepId, triggerID, rawTrigger, upkeep)) return false;
} else if (triggerType == Trigger.LOG) {
if (!_validateLogTrigger(upkeepId, rawTrigger)) return false;
if (!_validateLogTrigger(upkeepId, triggerID, rawTrigger)) return false;
} else {
revert InvalidTriggerType();
}
if (upkeep.maxValidBlocknumber <= _blockNum()) {
// Can happen when an upkeep got cancelled after report was generated.
// However we have a CANCELLATION_DELAY of 50 blocks so shouldn't happen in practice
emit CancelledUpkeepReport(upkeepId, rawTrigger);
emit CancelledUpkeepReport(upkeepId, triggerID);
return false;
}

if (upkeep.balance < maxLinkPayment) {
// Can happen due to flucutations in gas / link prices
emit InsufficientFundsUpkeepReport(upkeepId, rawTrigger);
emit InsufficientFundsUpkeepReport(upkeepId, triggerID);
return false;
}

Expand All @@ -435,13 +438,14 @@ contract KeeperRegistry2_1 is KeeperRegistryBase2_1, OCR2Abstract, Chainable, ER
*/
function _validateConditionalTrigger(
uint256 upkeepId,
bytes32 triggerID,
bytes memory rawTrigger,
Upkeep memory upkeep
) internal returns (bool) {
ConditionalTrigger memory trigger = abi.decode(rawTrigger, (ConditionalTrigger));
if (trigger.blockNum < upkeep.lastPerformedBlockNumber) {
// Can happen when another report performed this upkeep after this report was generated
emit StaleUpkeepReport(upkeepId, rawTrigger);
emit StaleUpkeepReport(upkeepId, triggerID);
return false;
}
if (
Expand All @@ -454,28 +458,28 @@ contract KeeperRegistry2_1 is KeeperRegistryBase2_1, OCR2Abstract, Chainable, ER
// 2. blockHash at trigger block number was same as trigger time. This is an optional check which is
// applied if DON sends non empty trigger.blockHash. Note: It only works for last 256 blocks on chain
// when it is sent
emit ReorgedUpkeepReport(upkeepId, rawTrigger);
emit ReorgedUpkeepReport(upkeepId, triggerID);
return false;
}
return true;
}

function _validateLogTrigger(uint256 upkeepId, bytes memory rawTrigger) internal returns (bool) {
function _validateLogTrigger(uint256 upkeepId, bytes32 triggerID, bytes memory rawTrigger) internal returns (bool) {
LogTrigger memory trigger = abi.decode(rawTrigger, (LogTrigger));
if (
(trigger.blockHash != bytes32("") && _blockHash(trigger.blockNum) != trigger.blockHash) ||
trigger.blockNum >= _blockNum()
) {
// Reorg protection is same as conditional trigger upkeeps
emit ReorgedUpkeepReport(upkeepId, rawTrigger);
emit ReorgedUpkeepReport(upkeepId, triggerID);
return false;
}
bytes32 logTriggerID = keccak256(abi.encodePacked(upkeepId, trigger.txHash, trigger.logIndex));
if (s_observedLogTriggers[logTriggerID]) {
emit StaleUpkeepReport(upkeepId, rawTrigger);
bytes32 logDedupID = keccak256(abi.encodePacked(upkeepId, trigger.txHash, trigger.logIndex));
if (s_observedLogTriggers[logDedupID]) {
emit StaleUpkeepReport(upkeepId, triggerID);
return false;
}
s_observedLogTriggers[logTriggerID] = true;
s_observedLogTriggers[logDedupID] = true;
return true;
}

Expand Down
21 changes: 16 additions & 5 deletions contracts/src/v0.8/dev/automation/2_1/KeeperRegistryBase2_1.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,7 @@ abstract contract KeeperRegistryBase2_1 is ConfirmedOwner, ExecutionPrevention {
Trigger triggerType;
uint256 gasUsed;
uint256 gasOverhead;
bytes32 triggerID;
}

struct Transmitter {
Expand Down Expand Up @@ -433,15 +434,15 @@ abstract contract KeeperRegistryBase2_1 is ConfirmedOwner, ExecutionPrevention {
uint96 totalPayment,
uint256 gasUsed,
uint256 gasOverhead,
bytes trigger
bytes32 triggerID
);
event UpkeepReceived(uint256 indexed id, uint256 startingBalance, address importedFrom);
event UpkeepUnpaused(uint256 indexed id);
event UpkeepRegistered(uint256 indexed id, uint32 executeGas, address admin);
event StaleUpkeepReport(uint256 indexed id, bytes trigger);
event ReorgedUpkeepReport(uint256 indexed id, bytes trigger);
event InsufficientFundsUpkeepReport(uint256 indexed id, bytes trigger);
event CancelledUpkeepReport(uint256 indexed id, bytes trigger);
event StaleUpkeepReport(uint256 indexed id, bytes32 triggerID);
event ReorgedUpkeepReport(uint256 indexed id, bytes32 triggerID);
event InsufficientFundsUpkeepReport(uint256 indexed id, bytes32 triggerID);
event CancelledUpkeepReport(uint256 indexed id, bytes32 triggerID);
event Paused(address account);
event Unpaused(address account);

Expand Down Expand Up @@ -714,6 +715,16 @@ abstract contract KeeperRegistryBase2_1 is ConfirmedOwner, ExecutionPrevention {
}
}

/**
* @notice returns a unique identifier for an upkeep/trigger combo
* @param upkeepID the upkeep id
* @param trigger the raw trigger bytes
* @return triggerID the unique identifier for the upkeep/trigger combo
*/
function _triggerID(uint256 upkeepID, bytes memory trigger) internal pure returns (bytes32 triggerID) {
return keccak256(abi.encodePacked(upkeepID, trigger));
}

/**
* @notice replicates Open Zeppelin's ReentrancyGuard but optimized to fit our storage
*/
Expand Down
12 changes: 6 additions & 6 deletions contracts/src/v0.8/dev/automation/2_1/interfaces/IKeeperRegistryMaster.sol
View file

Large diffs are not rendered by default.

52 changes: 26 additions & 26 deletions contracts/test/v0.8/automation/KeeperRegistry2_1.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ const cancellationDelay = 50

// This is the margin for gas that we test for. Gas charged should always be greater
// than total gas used in tx but should not increase beyond this margin
const gasCalculationMargin = BigNumber.from(5000)
const gasCalculationMargin = BigNumber.from(8000)

const linkEth = BigNumber.from(5000000000000000) // 1 Link = 0.005 Eth
const gasWei = BigNumber.from(1000000000) // 1 gwei
Expand Down Expand Up @@ -238,14 +238,6 @@ const encodeReport = (report: Report) => {
)
}

const decodeBlockTrigger = (trigger: BytesLike) => {
const [blockNum, blockHash] = ethers.utils.defaultAbiCoder.decode(
['tuple(uint32, bytes32)'],
trigger,
)[0] as [number, string]
return { blockNum, blockHash }
}

type UpkeepData = {
Id: BigNumberish
performGas: BigNumberish
Expand Down Expand Up @@ -317,7 +309,7 @@ const parseUpkeepPerformedLogs = (receipt: ContractReceipt) => {
if (
log.name ==
registry.interface.events[
'UpkeepPerformed(uint256,bool,uint96,uint256,uint256,bytes)'
'UpkeepPerformed(uint256,bool,uint96,uint256,uint256,bytes32)'
].name
) {
parsedLogs.push(log as unknown as UpkeepPerformedEvent)
Expand All @@ -336,7 +328,7 @@ const parseReorgedUpkeepReportLogs = (receipt: ContractReceipt) => {
const log = registry.interface.parseLog(rawLog)
if (
log.name ==
registry.interface.events['ReorgedUpkeepReport(uint256,bytes)'].name
registry.interface.events['ReorgedUpkeepReport(uint256,bytes32)'].name
) {
parsedLogs.push(log as unknown as ReorgedUpkeepReportEvent)
}
Expand All @@ -354,7 +346,7 @@ const parseStaleUpkeepReportLogs = (receipt: ContractReceipt) => {
const log = registry.interface.parseLog(rawLog)
if (
log.name ==
registry.interface.events['StaleUpkeepReport(uint256,bytes)'].name
registry.interface.events['StaleUpkeepReport(uint256,bytes32)'].name
) {
parsedLogs.push(log as unknown as StaleUpkeepReportEvent)
}
Expand All @@ -373,7 +365,7 @@ const parseInsufficientFundsUpkeepReportLogs = (receipt: ContractReceipt) => {
if (
log.name ==
registry.interface.events[
'InsufficientFundsUpkeepReport(uint256,bytes)'
'InsufficientFundsUpkeepReport(uint256,bytes32)'
].name
) {
parsedLogs.push(log as unknown as InsufficientFundsUpkeepReportEvent)
Expand All @@ -392,7 +384,7 @@ const parseCancelledUpkeepReportLogs = (receipt: ContractReceipt) => {
const log = registry.interface.parseLog(rawLog)
if (
log.name ==
registry.interface.events['CancelledUpkeepReport(uint256,bytes)'].name
registry.interface.events['CancelledUpkeepReport(uint256,bytes32)'].name
) {
parsedLogs.push(log as unknown as CancelledUpkeepReportEvent)
}
Expand Down Expand Up @@ -1768,8 +1760,8 @@ describe('KeeperRegistry2_1', () => {

// Do the thing
const tx = await getTransmitTx(registry, keeper1, [upkeepId], {
checkBlockNum: checkBlock.number - 1,
checkBlockHash: checkBlock.parentHash,
checkBlockNum: checkBlock.number,
checkBlockHash: checkBlock.hash,
numSigners: newF + 1,
})

Expand All @@ -1782,17 +1774,25 @@ describe('KeeperRegistry2_1', () => {

const id = upkeepPerformedLog.args.id
const success = upkeepPerformedLog.args.success
const { blockNum, blockHash } = decodeBlockTrigger(
upkeepPerformedLog.args.trigger,
)
const triggerID = upkeepPerformedLog.args.triggerID
const gasUsed = upkeepPerformedLog.args.gasUsed
const gasOverhead = upkeepPerformedLog.args.gasOverhead
const totalPayment = upkeepPerformedLog.args.totalPayment

assert.equal(id.toString(), upkeepId.toString())
assert.equal(success, true)
assert.equal(blockNum, checkBlock.number - 1)
assert.equal(blockHash, checkBlock.parentHash)
assert.equal(
triggerID,
ethers.utils.keccak256(
ethers.utils.concat([
ethers.utils.defaultAbiCoder.encode(['uint256'], [upkeepId]),
encodeBlockTrigger({
blockNum: checkBlock.number,
blockHash: checkBlock.hash,
}),
]),
),
'incorrect calculation of triggerID',
)
assert.isTrue(gasUsed.gt(BigNumber.from('0')))
assert.isTrue(gasOverhead.gt(BigNumber.from('0')))
assert.isTrue(totalPayment.gt(BigNumber.from('0')))
Expand Down Expand Up @@ -1958,12 +1958,12 @@ describe('KeeperRegistry2_1', () => {
assert.isTrue(
chargedGasOverhead
.sub(actualGasOverhead)
.lt(BigNumber.from(gasCalculationMargin)),
.lt(gasCalculationMargin),
),
'Gas overhead calculated is too high, decrease account gas variables (ACCOUNTING_FIXED_GAS_OVERHEAD/ACCOUNTING_PER_SIGNER_GAS_OVERHEAD) by atleast ' +
chargedGasOverhead
.sub(chargedGasOverhead)
.sub(BigNumber.from(gasCalculationMargin))
.sub(gasCalculationMargin)
.toString()
}
}
Expand Down Expand Up @@ -2063,12 +2063,12 @@ describe('KeeperRegistry2_1', () => {
assert.isTrue(
chargedGasOverhead
.sub(actualGasOverhead)
.lt(BigNumber.from(gasCalculationMargin)),
.lt(gasCalculationMargin),
),
'Gas overhead calculated is too high, decrease account gas variables (ACCOUNTING_FIXED_GAS_OVERHEAD/ACCOUNTING_PER_SIGNER_GAS_OVERHEAD) by atleast ' +
chargedGasOverhead
.sub(chargedGasOverhead)
.sub(BigNumber.from(gasCalculationMargin))
.sub(gasCalculationMargin)
.toString()
},
)
Expand Down
38 changes: 19 additions & 19 deletions ...rs/generated/i_keeper_registry_master_wrapper_2_1/i_keeper_registry_master_wrapper_2_1.go
View file

Large diffs are not rendered by default.

38 changes: 19 additions & 19 deletions ...pers/generated/keeper_registry_logic_a_wrapper_2_1/keeper_registry_logic_a_wrapper_2_1.go
View file

Large diffs are not rendered by default.

38 changes: 19 additions & 19 deletions ...pers/generated/keeper_registry_logic_b_wrapper_2_1/keeper_registry_logic_b_wrapper_2_1.go
View file

Large diffs are not rendered by default.

40 changes: 20 additions & 20 deletions core/gethwrappers/generated/keeper_registry_wrapper_2_1/keeper_registry_wrapper_2_1.go
View file

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions core/gethwrappers/generation/generated-wrapper-dependency-versions-do-not-edit.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,19 @@ flux_aggregator_wrapper: ../../contracts/solc/v0.6/FluxAggregator.abi ../../cont
functions_billing_registry_events_mock: ../../contracts/solc/v0.8.6/FunctionsBillingRegistryEventsMock.abi ../../contracts/solc/v0.8.6/FunctionsBillingRegistryEventsMock.bin 50deeb883bd9c3729702be335c0388f9d8553bab4be5e26ecacac496a89e2b77
functions_oracle_events_mock: ../../contracts/solc/v0.8.6/FunctionsOracleEventsMock.abi ../../contracts/solc/v0.8.6/FunctionsOracleEventsMock.bin 3ca70f966f8fe751987f0ccb50bebb6aa5be77e4a9f835d1ae99e0e9bfb7d52c
gas_wrapper: ../../contracts/solc/v0.8.6/KeeperRegistryCheckUpkeepGasUsageWrapper1_2.abi ../../contracts/solc/v0.8.6/KeeperRegistryCheckUpkeepGasUsageWrapper1_2.bin 4a5dcdac486d18fcd58e3488c15c1710ae76b977556a3f3191bd269a4bc75723
i_keeper_registry_master_wrapper_2_1: ../../contracts/solc/v0.8.16/IKeeperRegistryMaster.abi ../../contracts/solc/v0.8.16/IKeeperRegistryMaster.bin 4b04264dd367c040786de7cd7ba1691f1c99cbfa039d2aafcb38b28cb41dd89e
i_keeper_registry_master_wrapper_2_1: ../../contracts/solc/v0.8.16/IKeeperRegistryMaster.abi ../../contracts/solc/v0.8.16/IKeeperRegistryMaster.bin f15f7d932bea88d23a8658a9c5d345ce39c9b81bc5923e0b4fd96c78d672c4a7
i_log_automation: ../../contracts/solc/v0.8.16/ILogAutomation.abi ../../contracts/solc/v0.8.16/ILogAutomation.bin 8cc5fbd18810dd2a3dc12871eabdb3bf79634544614e00ee967382767b9dfde3
keeper_registrar_wrapper1_2: ../../contracts/solc/v0.8.6/KeeperRegistrar.abi ../../contracts/solc/v0.8.6/KeeperRegistrar.bin e49b2f8b23da17af1ed2209b8ae0968cc04350554d636711e6c24a3ad3118692
keeper_registrar_wrapper2_0: ../../contracts/solc/v0.8.6/KeeperRegistrar2_0.abi ../../contracts/solc/v0.8.6/KeeperRegistrar2_0.bin 647f125c2f0dafabcdc545cb77b15dc2ec3ea9429357806813179b1fd555c2d2
keeper_registry_logic1_3: ../../contracts/solc/v0.8.6/KeeperRegistryLogic1_3.abi ../../contracts/solc/v0.8.6/KeeperRegistryLogic1_3.bin e1bee66ce7cd0085469f923c46f0eddb58fd45dec207def1bb383b37e413a6ca
keeper_registry_logic2_0: ../../contracts/solc/v0.8.6/KeeperRegistryLogic2_0.abi ../../contracts/solc/v0.8.6/KeeperRegistryLogic2_0.bin ba5c23c495c4e1e487560ed56d917632f0047266c06fda4af9edbcda5aca99fa
keeper_registry_logic_a_wrapper_2_1: ../../contracts/solc/v0.8.16/KeeperRegistryLogicA2_1.abi ../../contracts/solc/v0.8.16/KeeperRegistryLogicA2_1.bin 291df07cfa8d6dd2392c6c7425b0fbb0e0b0cf8a9facfde0c17fc0135ee16b83
keeper_registry_logic_b_wrapper_2_1: ../../contracts/solc/v0.8.16/KeeperRegistryLogicB2_1.abi ../../contracts/solc/v0.8.16/KeeperRegistryLogicB2_1.bin ef97a4c6fa4552b2b19cbfec688253941ec4b5ad27879cddb93183bf829d7e4a
keeper_registry_logic_a_wrapper_2_1: ../../contracts/solc/v0.8.16/KeeperRegistryLogicA2_1.abi ../../contracts/solc/v0.8.16/KeeperRegistryLogicA2_1.bin 125277676b8a461cab0696a7db50dc562aa17b254fcfa92dd22654839c00a273
keeper_registry_logic_b_wrapper_2_1: ../../contracts/solc/v0.8.16/KeeperRegistryLogicB2_1.abi ../../contracts/solc/v0.8.16/KeeperRegistryLogicB2_1.bin ceabe7dbab34ca6925da57c4898a95e5e23bf8c92d7278e4d2d2e554869e5c37
keeper_registry_wrapper1_1: ../../contracts/solc/v0.7/KeeperRegistry1_1.abi ../../contracts/solc/v0.7/KeeperRegistry1_1.bin 6ce079f2738f015f7374673a2816e8e9787143d00b780ea7652c8aa9ad9e1e20
keeper_registry_wrapper1_2: ../../contracts/solc/v0.8.6/KeeperRegistry1_2.abi ../../contracts/solc/v0.8.6/KeeperRegistry1_2.bin 41faf687ad6a5171cc91e627244d0b3d6f62d393c418ca22d4ba7fc921fd32c6
keeper_registry_wrapper1_3: ../../contracts/solc/v0.8.6/KeeperRegistry1_3.abi ../../contracts/solc/v0.8.6/KeeperRegistry1_3.bin 5e1414eacbc1880b7349a4f253b7eca176f7f6300ef3cd834c493ce795a17e25
keeper_registry_wrapper2_0: ../../contracts/solc/v0.8.6/KeeperRegistry2_0.abi ../../contracts/solc/v0.8.6/KeeperRegistry2_0.bin c32dea7d5ef66b7c58ddc84ddf69aa44df1b3ae8601fbc271c95be4ff5853056
keeper_registry_wrapper_2_1: ../../contracts/solc/v0.8.16/KeeperRegistry2_1.abi ../../contracts/solc/v0.8.16/KeeperRegistry2_1.bin 29b8d9daa13afd6f965635173175fde183cb239f65a088a25f650861639af2f0
keeper_registry_wrapper_2_1: ../../contracts/solc/v0.8.16/KeeperRegistry2_1.abi ../../contracts/solc/v0.8.16/KeeperRegistry2_1.bin 52b89741b831b117a1af7f66f454e29b826f07777e3a6cb06362dd311a1fb948
keepers_vrf_consumer: ../../contracts/solc/v0.8.6/KeepersVRFConsumer.abi ../../contracts/solc/v0.8.6/KeepersVRFConsumer.bin fa75572e689c9e84705c63e8dbe1b7b8aa1a8fe82d66356c4873d024bb9166e8
llo_feeds: ../../contracts/solc/v0.8.16/VerifierProxy.abi ../../contracts/solc/v0.8.16/VerifierProxy.bin 3b69ffe9c694e8551b5375c02b9e960adc985e2390566740e7fea70c89e436f1
llo_feeds_test: ../../contracts/solc/v0.8.16/ExposedVerifier.abi ../../contracts/solc/v0.8.16/ExposedVerifier.bin 6932cea8f2738e874d3ec9e1a4231d2421704030c071d9e15dd2f7f08482c246
Expand Down

0 comments on commit 84ee2b5

Please sign in to comment.