Skip to content

Commit

Permalink
Fix batching tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@infiloop2
infiloop2 committed Feb 16, 2024
1 parent 9915be9 commit 7b43985
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 96 deletions.
1 change: 1 addition & 0 deletions contracts/src/v0.8/automation/dev/v2_2/AutomationRegistry2_2.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ contract AutomationRegistry2_2 is AutomationRegistryBase2_2, OCR2Abstract, Chain
report.fastGasWei,
report.linkNative,
gasOverhead,
// TODO: Divide gas overhead in proportion to performData
l1Fee / transmitVars.numUpkeepsPassedChecks
);
transmitVars.totalPremium += premium;
Expand Down
4 changes: 2 additions & 2 deletions contracts/src/v0.8/automation/dev/v2_2/AutomationRegistryBase2_2.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ abstract contract AutomationRegistryBase2_2 is ConfirmedOwner {
// tx itself, but since payment processing itself takes gas, and it needs the overhead as input, we use fixed constants
// to account for gas used in payment processing. These values are calibrated using hardhat tests which simulates various cases and verifies that
// the variables result in accurate estimation
uint256 internal constant ACCOUNTING_FIXED_GAS_OVERHEAD = 23_000; // Fixed overhead per tx
uint256 internal constant ACCOUNTING_PER_UPKEEP_GAS_OVERHEAD = 6_000; // Overhead per upkeep performed in batch
uint256 internal constant ACCOUNTING_FIXED_GAS_OVERHEAD = 22_500; // Fixed overhead per tx
uint256 internal constant ACCOUNTING_PER_UPKEEP_GAS_OVERHEAD = 6_500; // Overhead per upkeep performed in batch

LinkTokenInterface internal immutable i_link;
AggregatorV3Interface internal immutable i_linkNativeFeed;
Expand Down
121 changes: 27 additions & 94 deletions contracts/test/v0.8/automation/AutomationRegistry2_2.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ const emptyBytes32 =
'0x0000000000000000000000000000000000000000000000000000000000000000'

const transmitGasOverhead = 1_000_000
const checkGasOverhead = 400_000 + 4_000 // 4_000 for the overhead to call chain module
const checkGasOverhead = 500_000

const stalenessSeconds = BigNumber.from(43820)
const gasCeilingMultiplier = BigNumber.from(2)
Expand Down Expand Up @@ -2149,7 +2149,7 @@ describe('AutomationRegistry2_2', () => {
})
})

describe.skip('#transmit with upkeep batches [ @skip-coverage ]', function () {
describe('#transmit with upkeep batches [ @skip-coverage ]', function () {
const numPassingConditionalUpkeepsArray = [0, 1, 5]
const numPassingLogUpkeepsArray = [0, 1, 5]
const numFailingUpkeepsArray = [0, 3]
Expand Down Expand Up @@ -2454,110 +2454,49 @@ describe('AutomationRegistry2_2', () => {
numPassingConditionalUpkeeps + numPassingLogUpkeeps,
)

const chainModuleOverheads =
await chainModuleBase.getGasOverhead()
const gasConditionalOverheadCap = registryConditionalOverhead
//.add(registryPerSignerGasOverhead.mul(BigNumber.from(f + 1)))
.add(
registryPerPerformByteGasOverhead
.add(chainModuleOverheads.chainModulePerByteOverhead)
.mul(
maxPerformDataSize
.add(registryTransmitCalldataFixedBytesOverhead)
.add(
registryTransmitCalldataPerSignerBytesOverhead.mul(
BigNumber.from(f + 1),
),
),
),
)
.add(chainModuleOverheads.chainModuleFixedOverhead)
const gasLogOverheadCap = registryLogOverhead
//.add(registryPerSignerGasOverhead.mul(BigNumber.from(f + 1)))
.add(
registryPerPerformByteGasOverhead
.add(chainModuleOverheads.chainModulePerByteOverhead)
.mul(
maxPerformDataSize
.add(registryTransmitCalldataFixedBytesOverhead)
.add(
registryTransmitCalldataPerSignerBytesOverhead.mul(
BigNumber.from(f + 1),
),
),
),
)
.add(chainModuleOverheads.chainModuleFixedOverhead)

const overheadCanGetCapped =
numFailingUpkeeps > 0 &&
numPassingConditionalUpkeeps <= 1 &&
numPassingLogUpkeeps <= 1
// Can happen if there are failing upkeeps and only 1 successful upkeep of each type
let netGasUsedPlusOverhead = BigNumber.from('0')

let netGasUsedPlusChargedOverhead = BigNumber.from('0')
for (let i = 0; i < numPassingConditionalUpkeeps; i++) {
const gasUsed = upkeepPerformedLogs[i].args.gasUsed
const gasOverhead = upkeepPerformedLogs[i].args.gasOverhead
const chargedGasOverhead =
upkeepPerformedLogs[i].args.gasOverhead

assert.isTrue(gasUsed.gt(BigNumber.from('0')))
assert.isTrue(gasOverhead.gt(BigNumber.from('0')))

// Overhead should not exceed capped
assert.isTrue(gasOverhead.lte(gasConditionalOverheadCap))
assert.isTrue(chargedGasOverhead.gt(BigNumber.from('0')))

// Overhead should be same for every upkeep since they have equal performData, hence same caps
// Overhead should be same for every upkeep
assert.isTrue(
gasOverhead.eq(upkeepPerformedLogs[0].args.gasOverhead),
chargedGasOverhead.eq(
upkeepPerformedLogs[0].args.gasOverhead,
),
)

netGasUsedPlusOverhead = netGasUsedPlusOverhead
netGasUsedPlusChargedOverhead = netGasUsedPlusChargedOverhead
.add(gasUsed)
.add(gasOverhead)
.add(chargedGasOverhead)
}

for (let i = 0; i < numPassingLogUpkeeps; i++) {
const gasUsed =
upkeepPerformedLogs[numPassingConditionalUpkeeps + i].args
.gasUsed
const gasOverhead =
const chargedGasOverhead =
upkeepPerformedLogs[numPassingConditionalUpkeeps + i].args
.gasOverhead

assert.isTrue(gasUsed.gt(BigNumber.from('0')))
assert.isTrue(gasOverhead.gt(BigNumber.from('0')))

// Overhead should not exceed capped
assert.isTrue(gasOverhead.lte(gasLogOverheadCap))
assert.isTrue(chargedGasOverhead.gt(BigNumber.from('0')))

// Overhead should be same for every upkeep since they have equal performData, hence same caps
// Overhead should be same for every upkeep
assert.isTrue(
gasOverhead.eq(
chargedGasOverhead.eq(
upkeepPerformedLogs[numPassingConditionalUpkeeps].args
.gasOverhead,
),
)

netGasUsedPlusOverhead = netGasUsedPlusOverhead
netGasUsedPlusChargedOverhead = netGasUsedPlusChargedOverhead
.add(gasUsed)
.add(gasOverhead)
.add(chargedGasOverhead)
}

const overheadsGotCapped =
(numPassingConditionalUpkeeps > 0 &&
upkeepPerformedLogs[0].args.gasOverhead.eq(
gasConditionalOverheadCap,
)) ||
(numPassingLogUpkeeps > 0 &&
upkeepPerformedLogs[
numPassingConditionalUpkeeps
].args.gasOverhead.eq(gasLogOverheadCap))
// Should only get capped in certain scenarios
if (overheadsGotCapped) {
assert.isTrue(
overheadCanGetCapped,
'Gas overhead got capped. Verify gas overhead variables in test match those in the registry. To not have the overheads capped increase REGISTRY_GAS_OVERHEAD',
)
}

Check failure on line 2500 in contracts/test/v0.8/automation/AutomationRegistry2_2.test.ts

View workflow job for this annotation

GitHub Actions / Solidity Lint 

Delete `⏎`
console.log(
'Gas Benchmarking - batching (passedConditionalUpkeeps: ',
Expand All @@ -2567,33 +2506,27 @@ describe('AutomationRegistry2_2', () => {
'failedUpkeeps:',
numFailingUpkeeps,
'): ',
'overheadsGotCapped',
overheadsGotCapped,
numPassingConditionalUpkeeps > 0
? 'calculated conditional overhead'
? 'charged conditional overhead'
: '',
numPassingConditionalUpkeeps > 0
? upkeepPerformedLogs[0].args.gasOverhead.toString()
: '',
numPassingLogUpkeeps > 0 ? 'calculated log overhead' : '',
numPassingLogUpkeeps > 0 ? 'charged log overhead' : '',
numPassingLogUpkeeps > 0
? upkeepPerformedLogs[
numPassingConditionalUpkeeps
].args.gasOverhead.toString()
: '',
' margin over gasUsed',
netGasUsedPlusOverhead.sub(receipt.gasUsed).toString(),
netGasUsedPlusChargedOverhead.sub(receipt.gasUsed).toString(),
)

// If overheads don't get capped then total gas charged should be greater than tx gas
// We don't check whether the net is within gasMargin as the margin changes with numFailedUpkeeps
// Which is ok, as long as individual gas overhead is capped
if (!overheadsGotCapped) {
assert.isTrue(
netGasUsedPlusOverhead.gt(receipt.gasUsed),
'Gas overhead is too low, increase ACCOUNTING_PER_UPKEEP_GAS_OVERHEAD',
)
}
// The total gas charged should be greater than tx gas
assert.isTrue(
netGasUsedPlusChargedOverhead.gt(receipt.gasUsed),
'Charged gas overhead is too low for batch upkeeps, increase ACCOUNTING_PER_UPKEEP_GAS_OVERHEAD',
)
},
)
}
Expand Down

0 comments on commit 7b43985

Please sign in to comment.