Skip to content

Commit

Permalink
fix overhead tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@infiloop2
infiloop2 committed Feb 16, 2024
1 parent 21a03fe commit 9915be9
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 46 deletions.
8 changes: 4 additions & 4 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 @@ -48,9 +48,9 @@ abstract contract AutomationRegistryBase2_2 is ConfirmedOwner {
// Next block of constants are only used in maxPayment estimation during checkUpkeep simulation
// These values are calibrated using hardhat tests which simulates various cases and verifies that
// the variables result in accurate estimation
uint256 internal constant REGISTRY_CONDITIONAL_OVERHEAD = 60_000; // Fixed gas overhead for conditional upkeeps
uint256 internal constant REGISTRY_LOG_OVERHEAD = 110_400; // Fixed gas overhead for log upkeeps
uint256 internal constant REGISTRY_PER_SIGNER_GAS_OVERHEAD = 7_000; // Value scales with f
uint256 internal constant REGISTRY_CONDITIONAL_OVERHEAD = 62_000; // Fixed gas overhead for conditional upkeeps
uint256 internal constant REGISTRY_LOG_OVERHEAD = 87_000; // Fixed gas overhead for log upkeeps
uint256 internal constant REGISTRY_PER_SIGNER_GAS_OVERHEAD = 5_800; // Value scales with f
uint256 internal constant REGISTRY_PER_PERFORM_BYTE_GAS_OVERHEAD = 22; // Per perform data byte overhead
// The overhead (in bytes) in addition to perform data for upkeep sent in calldata
// This includes overhead for all struct encoding as well as report signatures
Expand All @@ -62,7 +62,7 @@ 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 = 20_000; // Fixed overhead per tx
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

LinkTokenInterface internal immutable i_link;
Expand Down
96 changes: 54 additions & 42 deletions contracts/test/v0.8/automation/AutomationRegistry2_2.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1996,33 +1996,33 @@ describe('AutomationRegistry2_2', () => {
chargedGasOverhead
.sub(actualGasOverhead)
.lt(gasCalculationMargin),
),
'Gas overhead calculated is too high, decrease account gas variables (ACCOUNTING_FIXED_GAS_OVERHEAD/ACCOUNTING_PER_SIGNER_GAS_OVERHEAD) by at least ' +
chargedGasOverhead
.sub(actualGasOverhead)
.sub(gasCalculationMargin)
.toString()
.toString(),
)

// The estimated overhead during checkUpkeep should be close to the actual overhead in transaction
// It should be greater than the actual overhead but not by a lot
// The estimated overhead is controlled by variables
// REGISTRY_CONDITIONAL_OVERHEAD, REGISTRY_LOG_OVERHEAD, REGISTRY_PER_SIGNER_GAS_OVERHEAD
// REGISTRY_PER_PERFORM_BYTE_GAS_OVERHEAD and TRANSMIT_CALLDATA_BYTES_OVERHEAD
// REGISTRY_PER_PERFORM_BYTE_GAS_OVERHEAD
assert.isTrue(
estimatedGasOverhead.gt(actualGasOverhead),
'Gas overhead estimated in check upkeep is too low, increase estimation gas variables (REGISTRY_CONDITIONAL_OVERHEAD/REGISTRY_LOG_OVERHEAD/REGISTRY_PER_SIGNER_GAS_OVERHEAD/REGISTRY_PER_PERFORM_BYTE_GAS_OVERHEAD/TRANSMIT_CALLDATA_BYTES_OVERHEAD) by at least ' +
'Gas overhead estimated in check upkeep is too low, increase estimation gas variables (REGISTRY_CONDITIONAL_OVERHEAD/REGISTRY_LOG_OVERHEAD/REGISTRY_PER_SIGNER_GAS_OVERHEAD/REGISTRY_PER_PERFORM_BYTE_GAS_OVERHEAD) by at least ' +
estimatedGasOverhead.sub(chargedGasOverhead).toString(),
)
assert.isTrue(
estimatedGasOverhead
.sub(actualGasOverhead)
.lt(gasEstimationMargin),
),
'Gas overhead estimated is too high, decrease estimation gas variables (REGISTRY_CONDITIONAL_OVERHEAD/REGISTRY_LOG_OVERHEAD/REGISTRY_PER_SIGNER_GAS_OVERHEAD/REGISTRY_PER_PERFORM_BYTE_GAS_OVERHEAD/TRANSMIT_CALLDATA_BYTES_OVERHEAD) by at least ' +
'Gas overhead estimated is too high, decrease estimation gas variables (REGISTRY_CONDITIONAL_OVERHEAD/REGISTRY_LOG_OVERHEAD/REGISTRY_PER_SIGNER_GAS_OVERHEAD/REGISTRY_PER_PERFORM_BYTE_GAS_OVERHEAD) by at least ' +
estimatedGasOverhead
.sub(actualGasOverhead)
.sub(gasEstimationMargin)
.toString()
.toString(),
)
}
}
}
Expand All @@ -2031,7 +2031,7 @@ describe('AutomationRegistry2_2', () => {
})
})

describe.skip('Gas benchmarking log upkeeps [ @skip-coverage ]', function () {
describe('Gas benchmarking log upkeeps [ @skip-coverage ]', function () {
const fs = [1, 10]
fs.forEach(function (newF) {
it(
Expand Down Expand Up @@ -2062,15 +2062,32 @@ describe('AutomationRegistry2_2', () => {
// exactly 1 Upkeep Performed should be emitted
assert.equal(upkeepPerformedLogs.length, 1)
const upkeepPerformedLog = upkeepPerformedLogs[0]
const chainModuleOverheads =
await chainModuleBase.getGasOverhead()

const upkeepGasUsed = upkeepPerformedLog.args.gasUsed
const chargedGasOverhead = upkeepPerformedLog.args.gasOverhead
const actualGasOverhead = receipt.gasUsed.sub(upkeepGasUsed)
const chainModuleOverheads =
await chainModuleBase.getGasOverhead()
const estimatedGasOverhead = registryLogOverhead
.add(registryPerSignerGasOverhead.mul(BigNumber.from(newF + 1)))
.add(
registryPerPerformByteGasOverhead
.add(chainModuleOverheads.chainModulePerByteOverhead)
.mul(
BigNumber.from(performData.length / 2 - 1)
.add(registryTransmitCalldataFixedBytesOverhead)
.add(
registryTransmitCalldataPerSignerBytesOverhead.mul(
BigNumber.from(newF + 1),
),
),
),
)
.add(chainModuleOverheads.chainModuleFixedOverhead)

assert.isTrue(upkeepGasUsed.gt(BigNumber.from('0')))
assert.isTrue(chargedGasOverhead.gt(BigNumber.from('0')))
assert.isTrue(actualGasOverhead.gt(BigNumber.from('0')))

console.log(
'Gas Benchmarking log upkeeps:',
Expand All @@ -2082,54 +2099,49 @@ describe('AutomationRegistry2_2', () => {
performData.length / 2 - 1,
'sig verification ( f =',
newF,
'): calculated overhead: ',
'): estimated overhead: ',
estimatedGasOverhead.toString(),
' charged overhead: ',
chargedGasOverhead.toString(),
' actual overhead: ',
actualGasOverhead.toString(),
' margin over gasUsed: ',
' calculation margin over gasUsed: ',
chargedGasOverhead.sub(actualGasOverhead).toString(),
' estimation margin over gasUsed: ',
estimatedGasOverhead.sub(actualGasOverhead).toString(),
)

// Overhead should not get capped
const gasOverheadCap = registryLogOverhead
.add(registryPerSignerGasOverhead.mul(BigNumber.from(newF + 1)))
.add(
registryPerPerformByteGasOverhead
.add(chainModuleOverheads.chainModulePerByteOverhead)
.mul(
BigNumber.from(performData.length)
.add(registryTransmitCalldataFixedBytesOverhead)
.add(
registryTransmitCalldataPerSignerBytesOverhead.mul(
BigNumber.from(newF + 1),
),
),
),
)
.add(chainModuleOverheads.chainModuleFixedOverhead)
const gasCapMinusOverhead = gasOverheadCap.sub(chargedGasOverhead)
assert.isTrue(
gasCapMinusOverhead.gt(BigNumber.from(0)),
'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 by atleast ' +
gasCapMinusOverhead.toString(),
)
// total gas charged should be greater than tx gas but within gasCalculationMargin
assert.isTrue(
chargedGasOverhead.gt(actualGasOverhead),
'Gas overhead calculated is too low, increase account gas variables (ACCOUNTING_FIXED_GAS_OVERHEAD/ACCOUNTING_PER_SIGNER_GAS_OVERHEAD) by atleast ' +
'Gas overhead calculated is too low, increase account gas variables (ACCOUNTING_FIXED_GAS_OVERHEAD/ACCOUNTING_PER_UPKEEP_GAS_OVERHEAD) by at least ' +
actualGasOverhead.sub(chargedGasOverhead).toString(),
)

assert.isTrue(
chargedGasOverhead
.sub(actualGasOverhead)
.lt(gasCalculationMargin),
),
'Gas overhead calculated is too high, decrease account gas variables (ACCOUNTING_FIXED_GAS_OVERHEAD/ACCOUNTING_PER_SIGNER_GAS_OVERHEAD) by atleast ' +
'Gas overhead calculated is too high, decrease account gas variables (ACCOUNTING_FIXED_GAS_OVERHEAD/ACCOUNTING_PER_SIGNER_GAS_OVERHEAD) by at least ' +
chargedGasOverhead
.sub(chargedGasOverhead)
.sub(actualGasOverhead)
.sub(gasCalculationMargin)
.toString()
.toString(),
)

assert.isTrue(
estimatedGasOverhead.gt(actualGasOverhead),
'Gas overhead estimated in check upkeep is too low, increase estimation gas variables (REGISTRY_CONDITIONAL_OVERHEAD/REGISTRY_LOG_OVERHEAD/REGISTRY_PER_SIGNER_GAS_OVERHEAD/REGISTRY_PER_PERFORM_BYTE_GAS_OVERHEAD) by at least ' +
estimatedGasOverhead.sub(chargedGasOverhead).toString(),
)
assert.isTrue(
estimatedGasOverhead
.sub(actualGasOverhead)
.lt(gasEstimationMargin),
'Gas overhead estimated is too high, decrease estimation gas variables (REGISTRY_CONDITIONAL_OVERHEAD/REGISTRY_LOG_OVERHEAD/REGISTRY_PER_SIGNER_GAS_OVERHEAD/REGISTRY_PER_PERFORM_BYTE_GAS_OVERHEAD) by at least ' +
estimatedGasOverhead
.sub(actualGasOverhead)
.sub(gasEstimationMargin)
.toString(),
)
},
)
})
Expand Down

0 comments on commit 9915be9

Please sign in to comment.