Skip to content

N
nebula
Commit 83a449c4 authored by Maurelian's avatar Maurelian Committed by Kelvin Fichter
Browse files
Options

feat(core-utils): Add absolute deviation options to expectApprox

parent cb7e3ae7
Hide whitespace changes
Inline Side-by-side
Showing with 242 additions and 54 deletions
.changeset/red-birds-kick.md 0 → 100644
View file @ 83a449c4
---
'@eth-optimism/core-utils': minor
---
Change the expectApprox interface to allow setting an absoluteexpected deviation range
integration-tests/test/native-eth.spec.ts
View file @ 83a449c4
......@@ -62,7 +62,7 @@ describe('Native ETH Integration Tests', async () => {
'0xFFFF'
)
// Expect gas to be less than or equal to the target plus 1%
expectApprox(gas, 6700060, { upperPercentDeviation: 1 })
expectApprox(gas, 6700060, { absoluteUpperDeviation: 1000 })
})
})
......@@ -213,17 +213,17 @@ describe('Native ETH Integration Tests', async () => {
expectApprox(
postBalances.l1BridgeBalance,
preBalances.l1BridgeBalance.sub(withdrawAmount),
{ upperPercentDeviation: 1 }
{ percentUpperDeviation: 1 }
)
expectApprox(
postBalances.l2UserBalance,
preBalances.l2UserBalance.sub(withdrawAmount.add(fee)),
{ upperPercentDeviation: 1 }
{ percentUpperDeviation: 1 }
)
expectApprox(
postBalances.l1UserBalance,
preBalances.l1UserBalance.add(withdrawAmount),
{ upperPercentDeviation: 1 }
{ percentUpperDeviation: 1 }
)
})
......
integration-tests/test/rpc.spec.ts
View file @ 83a449c4
......@@ -380,7 +380,7 @@ describe('Basic RPC tests', () => {
value: 0,
})
// Expect gas to be less than or equal to the target plus 1%
expectApprox(estimate, 21000, { upperPercentDeviation: 1 })
expectApprox(estimate, 21000, { percentUpperDeviation: 1 })
})
it('should fail for a reverting call transaction', async () => {
......
packages/contracts/test/contracts/L1/messaging/deposit.gas.spec.ts
View file @ 83a449c4
......@@ -2,6 +2,7 @@
import { ethers } from 'hardhat'
import { Signer, ContractFactory, Contract, constants } from 'ethers'
import { smoddit } from '@eth-optimism/smock'
import { expectApprox } from '@eth-optimism/core-utils'
/* Internal Imports */
import {
......@@ -155,6 +156,12 @@ describe('[GAS BENCHMARK] Depositing via the standard bridge', () => {
(((regenesis040Cost - gasUsed) / regenesis040Cost) * 100).toFixed(2) +
'%'
)
expectApprox(gasUsed, 154_247, {
absoluteUpperDeviation: 500,
// Assert a lower bound of 1% reduction on gas cost. If your tests are breaking because your
// contracts are too efficient, consider updating the target value!
percentLowerDeviation: 1,
})
// Sanity check that the message was enqueued.
expect(await CanonicalTransactionChain.getQueueLength()).to.equal(2)
})
......@@ -186,7 +193,12 @@ describe('[GAS BENCHMARK] Depositing via the standard bridge', () => {
(((regenesis040Cost - gasUsed) / regenesis040Cost) * 100).toFixed(2) +
'%'
)
expectApprox(gasUsed, 202_088, {
absoluteUpperDeviation: 500,
// Assert a lower bound of 1% reduction on gas cost. If your tests are breaking because your
// contracts are too efficient, consider updating the target value!
percentLowerDeviation: 1,
})
// Sanity check that the message was enqueued.
expect(await CanonicalTransactionChain.getQueueLength()).to.equal(3)
})
......
packages/contracts/test/contracts/L1/rollup/CanonicalTransactionChain.gas.spec.ts
View file @ 83a449c4
......@@ -171,10 +171,10 @@ describe('[GAS BENCHMARK] CanonicalTransactionChain', () => {
(gasUsed - fixedCalldataCost) / numTxs
)
expectApprox(gasUsed, 1_422_181, {
upperPercentDeviation: 0,
absoluteUpperDeviation: 1000,
// Assert a lower bound of 1% reduction on gas cost. If your tests are breaking because your
// contracts are too efficient, consider updating the target value!
lowerPercentDeviation: 1,
percentLowerDeviation: 1,
})
}).timeout(10_000_000)
......@@ -219,10 +219,10 @@ describe('[GAS BENCHMARK] CanonicalTransactionChain', () => {
(gasUsed - fixedCalldataCost) / numTxs
)
expectApprox(gasUsed, 1_632_687, {
upperPercentDeviation: 0,
absoluteUpperDeviation: 1000,
// Assert a lower bound of 1% reduction on gas cost. If your tests are breaking because your
// contracts are too efficient, consider updating the target value!
lowerPercentDeviation: 1,
percentLowerDeviation: 1,
})
}).timeout(10_000_000)
......@@ -276,7 +276,12 @@ describe('[GAS BENCHMARK] CanonicalTransactionChain', () => {
'Non-calldata overhead gas cost per transaction:',
(gasUsed - fixedCalldataCost) / numTxs
)
expectApprox(gasUsed, 1_293_611, { upperPercentDeviation: 0 })
expectApprox(gasUsed, 891_158, {
absoluteUpperDeviation: 1000,
// Assert a lower bound of 1% reduction on gas cost. If your tests are breaking because your
// contracts are too efficient, consider updating the target value!
percentLowerDeviation: 1,
})
}).timeout(10_000_000)
})
......@@ -304,11 +309,11 @@ describe('[GAS BENCHMARK] CanonicalTransactionChain', () => {
console.log('Benchmark complete.')
console.log('Gas used:', gasUsed)
expectApprox(gasUsed, 219_896, {
upperPercentDeviation: 0,
expectApprox(gasUsed, 218_203, {
absoluteUpperDeviation: 500,
// Assert a lower bound of 1% reduction on gas cost. If your tests are breaking because your
// contracts are too efficient, consider updating the target value!
lowerPercentDeviation: 1,
percentLowerDeviation: 1,
})
})
......@@ -326,11 +331,11 @@ describe('[GAS BENCHMARK] CanonicalTransactionChain', () => {
console.log('Benchmark complete.')
console.log('Gas used:', gasUsed)
expectApprox(gasUsed, 158_709, {
upperPercentDeviation: 0,
expectApprox(gasUsed, 157_822, {
absoluteUpperDeviation: 500,
// Assert a lower bound of 1% reduction on gas cost. If your tests are breaking because your
// contracts are too efficient, consider updating the target value!
lowerPercentDeviation: 1,
percentLowerDeviation: 1,
})
})
})
......
packages/core-utils/src/common/test-utils.ts
View file @ 83a449c4
import { expect } from 'chai'
import { BigNumber } from 'ethers'
interface percentDeviationRange {
upperPercentDeviation: number
lowerPercentDeviation?: number
interface deviationRanges {
percentUpperDeviation?: number
percentLowerDeviation?: number
absoluteUpperDeviation?: number
absoluteLowerDeviation?: number
}
/**
......@@ -12,30 +14,74 @@ interface percentDeviationRange {
export const expectApprox = (
actual: BigNumber | number,
target: BigNumber | number,
{ upperPercentDeviation, lowerPercentDeviation = 100 }: percentDeviationRange
{
percentUpperDeviation,
percentLowerDeviation,
absoluteUpperDeviation,
absoluteLowerDeviation,
}: deviationRanges
): void => {
actual = BigNumber.from(actual)
target = BigNumber.from(target)
const validDeviations =
upperPercentDeviation >= 0 &&
upperPercentDeviation <= 100 &&
lowerPercentDeviation >= 0 &&
lowerPercentDeviation <= 100
if (!validDeviations) {
// Ensure at least one deviation parameter is defined
const nonNullDeviations =
percentUpperDeviation ||
percentLowerDeviation ||
absoluteUpperDeviation ||
absoluteLowerDeviation
if (!nonNullDeviations) {
throw new Error(
'Upper and lower deviation percentage arguments should be between 0 and 100'
'Must define at least one parameter to limit the deviation of the actual value.'
)
}
const upper = target.mul(100 + upperPercentDeviation).div(100)
const lower = target.mul(100 - lowerPercentDeviation).div(100)
expect(
actual.lte(upper),
`Actual value (${actual}) is more than ${upperPercentDeviation}% greater than target (${target})`
).to.be.true
expect(
actual.gte(lower),
`Actual value (${actual}) is more than ${lowerPercentDeviation}% less than target (${target})`
).to.be.true
// Upper bound calculation.
let upper: BigNumber
// Set the two possible upper bounds if and only if they are defined.
const upperPcnt: BigNumber = !percentUpperDeviation
? null
: target.mul(100 + percentUpperDeviation).div(100)
const upperAbs: BigNumber = !absoluteUpperDeviation
? null
: target.add(absoluteUpperDeviation)
if (upperPcnt && upperAbs) {
// If both are set, take the lesser of the two upper bounds.
upper = upperPcnt.lte(upperAbs) ? upperPcnt : upperAbs
} else {
// Else take whichever is not undefined or set to null.
upper = upperPcnt || upperAbs
}
// Lower bound calculation.
let lower: BigNumber
// Set the two possible lower bounds if and only if they are defined.
const lowerPcnt: BigNumber = !percentLowerDeviation
? null
: target.mul(100 - percentLowerDeviation).div(100)
const lowerAbs: BigNumber = !absoluteLowerDeviation
? null
: target.sub(absoluteLowerDeviation)
if (lowerPcnt && lowerAbs) {
// If both are set, take the greater of the two lower bounds.
lower = lowerPcnt.gte(lowerAbs) ? lowerPcnt : lowerAbs
} else {
// Else take whichever is not undefined or set to null.
lower = lowerPcnt || lowerAbs
}
// Apply the assertions if they are non-null.
if (upper) {
expect(
actual.lte(upper),
`Actual value (${actual}) is greater than the calculated upper bound of (${upper})`
).to.be.true
}
if (lower) {
expect(
actual.gte(lower),
`Actual value (${actual}) is less than the calculated lower bound of (${lower})`
).to.be.true
}
}
packages/core-utils/test/common/test-utils.spec.ts
View file @ 83a449c4
......@@ -2,30 +2,150 @@ import { expect } from '../setup'
/* Imports: Internal */
import { expectApprox } from '../../src'
import { assert } from 'chai'
describe('expectApprox', async () => {
it('should throw an error if the actual value is higher than expected', async () => {
describe('expectApprox', () => {
it('should pass when the actual number is higher, but within the expected range of the target', async () => {
expectApprox(119, 100, {
percentUpperDeviation: 20,
percentLowerDeviation: 20,
absoluteUpperDeviation: 20,
absoluteLowerDeviation: 20,
})
})
it('should pass when the actual number is lower, but within the expected range of the target', async () => {
expectApprox(81, 100, {
percentUpperDeviation: 20,
percentLowerDeviation: 20,
absoluteUpperDeviation: 20,
absoluteLowerDeviation: 20,
})
})
it('should throw an error when no deviation values are given', async () => {
try {
expectApprox(121, 100, {
upperPercentDeviation: 20,
})
expectApprox(101, 100, {})
assert.fail('expectApprox did not throw an error')
} catch (error) {
expect(error.message).to.equal(
'Actual value (121) is more than 20% greater than target (100): expected false to be true'
'Must define at least one parameter to limit the deviation of the actual value.'
)
}
})
it('should throw an error if the actual value is lower than expected', async () => {
try {
expectApprox(79, 100, {
upperPercentDeviation: 0,
lowerPercentDeviation: 20,
describe('should throw an error if the actual value is higher than expected', () => {
describe('... when only one upper bound value is defined', () => {
it('... and percentUpperDeviation sets the upper bound', async () => {
try {
expectApprox(121, 100, {
percentUpperDeviation: 20,
})
assert.fail('expectApprox did not throw an error')
} catch (error) {
expect(error.message).to.equal(
'Actual value (121) is greater than the calculated upper bound of (120): expected false to be true'
)
}
})
} catch (error) {
expect(error.message).to.equal(
'Actual value (79) is more than 20% less than target (100): expected false to be true'
)
}
it('... and absoluteUpperDeviation sets the upper bound', async () => {
try {
expectApprox(121, 100, {
absoluteUpperDeviation: 20,
})
assert.fail('expectApprox did not throw an error')
} catch (error) {
expect(error.message).to.equal(
'Actual value (121) is greater than the calculated upper bound of (120): expected false to be true'
)
}
})
})
describe('... when both values are defined', () => {
it('... and percentUpperDeviation sets the upper bound', async () => {
try {
expectApprox(121, 100, {
percentUpperDeviation: 20,
absoluteUpperDeviation: 30,
})
assert.fail('expectApprox did not throw an error')
} catch (error) {
expect(error.message).to.equal(
'Actual value (121) is greater than the calculated upper bound of (120): expected false to be true'
)
}
})
it('... and absoluteUpperDeviation sets the upper bound', async () => {
try {
expectApprox(121, 100, {
percentUpperDeviation: 30,
absoluteUpperDeviation: 20,
})
assert.fail('expectApprox did not throw an error')
} catch (error) {
expect(error.message).to.equal(
'Actual value (121) is greater than the calculated upper bound of (120): expected false to be true'
)
}
})
})
})
describe('should throw an error if the actual value is lower than expected', () => {
describe('... when only one lower bound value is defined', () => {
it('... and percentLowerDeviation sets the lower bound', async () => {
try {
expectApprox(79, 100, {
percentLowerDeviation: 20,
})
assert.fail('expectApprox did not throw an error')
} catch (error) {
expect(error.message).to.equal(
'Actual value (79) is less than the calculated lower bound of (80): expected false to be true'
)
}
})
it('... and absoluteLowerDeviation sets the lower bound', async () => {
try {
expectApprox(79, 100, {
absoluteLowerDeviation: 20,
})
assert.fail('expectApprox did not throw an error')
} catch (error) {
expect(error.message).to.equal(
'Actual value (79) is less than the calculated lower bound of (80): expected false to be true'
)
}
})
})
describe('... when both values are defined', () => {
it('... and percentLowerDeviation sets the lower bound', async () => {
try {
expectApprox(79, 100, {
percentLowerDeviation: 20,
absoluteLowerDeviation: 30,
})
assert.fail('expectApprox did not throw an error')
} catch (error) {
expect(error.message).to.equal(
'Actual value (79) is less than the calculated lower bound of (80): expected false to be true'
)
}
})
it('... and absoluteLowerDeviation sets the lower bound', async () => {
try {
expectApprox(79, 100, {
percentLowerDeviation: 30,
absoluteLowerDeviation: 20,
})
assert.fail('expectApprox did not throw an error')
} catch (error) {
expect(error.message).to.equal(
'Actual value (79) is less than the calculated lower bound of (80): expected false to be true'
)
}
})
})
})
})
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment