Commit b6d1fcac authored by Maurelian's avatar Maurelian Committed by Kelvin Fichter

refactor(contracts): Combine two functions into one as setGasParams()

parent 53e4cae8
...@@ -87,7 +87,7 @@ contract CanonicalTransactionChain is ICanonicalTransactionChain, Lib_AddressRes ...@@ -87,7 +87,7 @@ contract CanonicalTransactionChain is ICanonicalTransactionChain, Lib_AddressRes
**********************/ **********************/
/** /**
* Modifier to enforce that, if configured, only the OVM_Sequencer contract may * Modifier to enforce that, if configured, only the Burn Admin may
* successfully call a method. * successfully call a method.
*/ */
modifier onlyBurnAdmin() { modifier onlyBurnAdmin() {
...@@ -103,36 +103,17 @@ contract CanonicalTransactionChain is ICanonicalTransactionChain, Lib_AddressRes ...@@ -103,36 +103,17 @@ contract CanonicalTransactionChain is ICanonicalTransactionChain, Lib_AddressRes
*******************************/ *******************************/
/** /**
* Allows the Sequencer to update the gas amount which is 'prepaid' during enqueue. * Allows the Burn Admin to update the parameters which determine the amount of gas to burn.
* The value of enqueueL2GasPrepaid is immediately updated as well. * The value of enqueueL2GasPrepaid is immediately updated as well.
*/ */
function setEnqueueGasCost(uint256 _enqueueGasCost) function setGasParams(uint256 _l2GasDiscountDivisor, uint256 _enqueueGasCost)
external external
onlyBurnAdmin onlyBurnAdmin
{ {
enqueueGasCost = _enqueueGasCost; enqueueGasCost = _enqueueGasCost;
// See the comment in enqueue() for the rationale behind this formula.
enqueueL2GasPrepaid = l2GasDiscountDivisor * _enqueueGasCost;
emit L2GasParamsUpdated(
l2GasDiscountDivisor,
enqueueGasCost,
enqueueL2GasPrepaid
);
}
/**
* Allows the Sequencer to update the L2 Gas Discount Divisor, which is defined as the ratio
* of the cost of gas on L1 to L2.
* The value of enqueueL2GasPrepaid is immediately updated as well.
*/
function setGasDivisor(uint256 _l2GasDiscountDivisor)
external
onlyBurnAdmin
{
l2GasDiscountDivisor = _l2GasDiscountDivisor; l2GasDiscountDivisor = _l2GasDiscountDivisor;
// See the comment in enqueue() for the rationale behind this formula. // See the comment in enqueue() for the rationale behind this formula.
enqueueL2GasPrepaid = _l2GasDiscountDivisor * enqueueGasCost; enqueueL2GasPrepaid = _l2GasDiscountDivisor * _enqueueGasCost;
emit L2GasParamsUpdated( emit L2GasParamsUpdated(
l2GasDiscountDivisor, l2GasDiscountDivisor,
...@@ -141,6 +122,7 @@ contract CanonicalTransactionChain is ICanonicalTransactionChain, Lib_AddressRes ...@@ -141,6 +122,7 @@ contract CanonicalTransactionChain is ICanonicalTransactionChain, Lib_AddressRes
); );
} }
/******************** /********************
* Public Functions * * Public Functions *
********************/ ********************/
......
...@@ -68,18 +68,10 @@ interface ICanonicalTransactionChain { ...@@ -68,18 +68,10 @@ interface ICanonicalTransactionChain {
*******************************/ *******************************/
/** /**
* Allows the Sequencer to update the gas amount which is 'prepaid' during enqueue. * Allows the Burn Admin to update the parameters which determine the amount of gas to burn.
* The value of enqueueL2GasPrepaid is immediately updated as well. * The value of enqueueL2GasPrepaid is immediately updated as well.
*/ */
function setEnqueueGasCost(uint256 _enqueueGasCost) function setGasParams(uint256 _l2GasDiscountDivisor, uint256 _enqueueGasCost)
external;
/**
* Allows the Sequencer to update the L2 Gas Discount Divisor, which is defined as the ratio
* of the cost of gas on L1 to L2.
* The value of enqueueL2GasPrepaid is immediately updated as well.
*/
function setGasDivisor(uint256 _l2GasDiscountDivisor)
external; external;
/******************** /********************
......
...@@ -146,7 +146,7 @@ describe('[GAS BENCHMARK] Depositing via the standard bridge', () => { ...@@ -146,7 +146,7 @@ describe('[GAS BENCHMARK] Depositing via the standard bridge', () => {
const receipt = await res.wait() const receipt = await res.wait()
const gasUsed = receipt.gasUsed.toNumber() const gasUsed = receipt.gasUsed.toNumber()
console.log(' - Gas used:', gasUsed) console.log(' - Gas used:', gasUsed)
expectApprox(gasUsed, 155_089, { expectApprox(gasUsed, 116_781, {
absoluteUpperDeviation: 500, absoluteUpperDeviation: 500,
// Assert a lower bound of 1% reduction on gas cost. If your tests are breaking because your // 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! // contracts are too efficient, consider updating the target value!
...@@ -173,7 +173,7 @@ describe('[GAS BENCHMARK] Depositing via the standard bridge', () => { ...@@ -173,7 +173,7 @@ describe('[GAS BENCHMARK] Depositing via the standard bridge', () => {
const receipt = await res.wait() const receipt = await res.wait()
const gasUsed = receipt.gasUsed.toNumber() const gasUsed = receipt.gasUsed.toNumber()
console.log(' - Gas used:', gasUsed) console.log(' - Gas used:', gasUsed)
expectApprox(gasUsed, 202_930, { expectApprox(gasUsed, 164_622, {
absoluteUpperDeviation: 500, absoluteUpperDeviation: 500,
// Assert a lower bound of 1% reduction on gas cost. If your tests are breaking because your // 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! // contracts are too efficient, consider updating the target value!
......
...@@ -293,7 +293,7 @@ describe('[GAS BENCHMARK] CanonicalTransactionChain', () => { ...@@ -293,7 +293,7 @@ describe('[GAS BENCHMARK] CanonicalTransactionChain', () => {
console.log('Benchmark complete.') console.log('Benchmark complete.')
expectApprox(gasUsed, 220_677, { expectApprox(gasUsed, 189_487, {
absoluteUpperDeviation: 500, absoluteUpperDeviation: 500,
// Assert a lower bound of 1% reduction on gas cost. If your tests are breaking because your // 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! // contracts are too efficient, consider updating the target value!
...@@ -314,7 +314,7 @@ describe('[GAS BENCHMARK] CanonicalTransactionChain', () => { ...@@ -314,7 +314,7 @@ describe('[GAS BENCHMARK] CanonicalTransactionChain', () => {
console.log('Benchmark complete.') console.log('Benchmark complete.')
expectApprox(gasUsed, 158_690, { expectApprox(gasUsed, 127_500, {
absoluteUpperDeviation: 500, absoluteUpperDeviation: 500,
// Assert a lower bound of 1% reduction on gas cost. If your tests are breaking because your // 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! // contracts are too efficient, consider updating the target value!
......
...@@ -137,63 +137,33 @@ describe('CanonicalTransactionChain', () => { ...@@ -137,63 +137,33 @@ describe('CanonicalTransactionChain', () => {
}) })
describe('Gas param setters', () => { describe('Gas param setters', () => {
describe('setGasDivisor', async () => { describe('setGasParams', async () => {
it('should revert when not called by the sequencer', async () => { it('should revert when not called by the Burn Admin', async () => {
await expect( await expect(
CanonicalTransactionChain.connect(otherSigner).setGasDivisor(32) CanonicalTransactionChain.connect(otherSigner).setGasParams(60000, 32)
).to.be.revertedWith('Only callable by the Burn Admin.')
})
it('should update the l2GasDiscountDivisor and enqueueL2GasPrepaid correctly', async () => {
const newGasDivisor = 19
await CanonicalTransactionChain.connect(
addressManagerOwner
).setGasDivisor(newGasDivisor)
const enqueueGasCost = await CanonicalTransactionChain.enqueueGasCost()
const enqueueL2GasPrepaid =
await CanonicalTransactionChain.enqueueL2GasPrepaid()
expect(enqueueL2GasPrepaid).to.equal(newGasDivisor * enqueueGasCost)
})
it('should emit an L2GasParamsUpdated event', async () => {
await expect(
CanonicalTransactionChain.connect(addressManagerOwner).setGasDivisor(
88
)
).to.emit(CanonicalTransactionChain, 'L2GasParamsUpdated')
})
})
describe('setEnqueueGasCost', async () => {
it('should revert when not called by the sequencer', async () => {
await expect(
CanonicalTransactionChain.connect(otherSigner).setEnqueueGasCost(
60000
)
).to.be.revertedWith('Only callable by the Burn Admin.') ).to.be.revertedWith('Only callable by the Burn Admin.')
}) })
it('should update the enqueueGasCost and enqueueL2GasPrepaid correctly', async () => { it('should update the enqueueGasCost and enqueueL2GasPrepaid correctly', async () => {
const newEnqueueGasCost = 31113 const newEnqueueGasCost = 31113
const newGasDivisor = 19
await CanonicalTransactionChain.connect( await CanonicalTransactionChain.connect(
addressManagerOwner addressManagerOwner
).setEnqueueGasCost(newEnqueueGasCost) ).setGasParams(newGasDivisor, newEnqueueGasCost)
const l2GasDiscountDivisor = const l2GasDiscountDivisor =
await CanonicalTransactionChain.l2GasDiscountDivisor() await CanonicalTransactionChain.l2GasDiscountDivisor()
const enqueueL2GasPrepaid = const enqueueL2GasPrepaid =
await CanonicalTransactionChain.enqueueL2GasPrepaid() await CanonicalTransactionChain.enqueueL2GasPrepaid()
expect(enqueueL2GasPrepaid).to.equal( expect(enqueueL2GasPrepaid).to.equal(newGasDivisor * newEnqueueGasCost)
l2GasDiscountDivisor * newEnqueueGasCost
)
}) })
it('should emit an L2GasParamsUpdated event', async () => { it('should emit an L2GasParamsUpdated event', async () => {
await expect( await expect(
CanonicalTransactionChain.connect( CanonicalTransactionChain.connect(addressManagerOwner).setGasParams(
addressManagerOwner 88,
).setEnqueueGasCost(31514) 31514
)
).to.emit(CanonicalTransactionChain, 'L2GasParamsUpdated') ).to.emit(CanonicalTransactionChain, 'L2GasParamsUpdated')
}) })
}) })
......
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