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.

Sign up for GitHub

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

Jump to bottom

replace trigger bytes with triggerID in logs #9712

Merged
merged 1 commit into from
Jul 10, 2023
Merged

replace trigger bytes with triggerID in logs #9712

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 17 additions & 13 deletions contracts/src/v0.8/dev/automation/2_1/KeeperRegistry2_1.sol
View file
Open in desktop
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
Open in desktop
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) {
infiloop2 marked this conversation as resolved.
Show resolved Hide resolved
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
Open in desktop

Large diffs are not rendered by default.

52 changes: 26 additions & 26 deletions contracts/test/v0.8/automation/KeeperRegistry2_1.test.ts
View file
Open in desktop
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
Open in desktop

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
Open in desktop

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
Open in desktop

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
Open in desktop

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
Open in desktop
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
Loading