Skip to content

Commit

Permalink
replace trigger bytes with triggerID in logs
Browse files Browse the repository at this point in the history
  • Loading branch information
@RyanRHall
RyanRHall committed Jun 23, 2023
1 parent 8062512 commit 05b2241
Show file tree
Hide file tree
Showing 9 changed files with 145 additions and 125 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 @@ -417,27 +419,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 @@ -449,13 +452,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 @@ -468,28 +472,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 >= block.number
) {
// 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.

41 changes: 23 additions & 18 deletions contracts/test/v0.8/automation/KeeperRegistry2_1.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,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 @@ -308,7 +308,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 @@ -327,7 +327,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 @@ -345,7 +345,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 @@ -364,7 +364,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 @@ -383,7 +383,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 @@ -1735,8 +1735,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 @@ -1749,17 +1749,22 @@ 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([
upkeepId.toHexString(),
encodeBlockTrigger(checkBlock.number, 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 @@ -1925,12 +1930,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 @@ -2030,12 +2035,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.

40 changes: 20 additions & 20 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.

40 changes: 20 additions & 20 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 @@ -17,19 +17,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 8c9629e0dc5ad39c45f7b088198862d927916c2b1d58d1febc51ffdc90a095e5
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 cc94482d5847359f7dfc399190f20b026fe8626f33a6a19db190911897bad0b3
keeper_registry_logic_b_wrapper_2_1: ../../contracts/solc/v0.8.16/KeeperRegistryLogicB2_1.abi ../../contracts/solc/v0.8.16/KeeperRegistryLogicB2_1.bin b5dadfbea0c738912c4fc6621f9205b9259feb42b10ed56be4fb5c963f2ec032
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 3ebee1f5ec17d70c1bf77109bf641b1e2a6cdf99c7149b63c7c05dee26a05721
keeper_registry_wrapper_2_1: ../../contracts/solc/v0.8.16/KeeperRegistry2_1.abi ../../contracts/solc/v0.8.16/KeeperRegistry2_1.bin 0b0e8635f30d33799819c7d8bac69748508e72edc727dbc4871f7f494299e13f
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 05b2241

Please sign in to comment.