Skip to content

Commit

Permalink
tx: invalidate cached dataFee on hardfork change
Browse files Browse the repository at this point in the history
  • Loading branch information
emersonmacro committed Oct 26, 2021
1 parent 9021108 commit abcfd9c
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 8 deletions.
4 changes: 2 additions & 2 deletions packages/tx/src/eip1559Transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -236,14 +236,14 @@ export default class FeeMarketEIP1559Transaction extends BaseTransaction<FeeMark
* The amount of gas paid for the data in this tx
*/
getDataFee(): BN {
if (Object.isFrozen(this) && this.cache.dataFee) {
if (this.cache.dataFee) {
return this.cache.dataFee
}

const cost = super.getDataFee()
cost.iaddn(AccessLists.getDataFeeEIP2930(this.accessList, this.common))

if (Object.isFrozen(this) && !this.cache.dataFee) {
if (Object.isFrozen(this)) {
this.cache.dataFee = cost
}

Expand Down
4 changes: 2 additions & 2 deletions packages/tx/src/eip2930Transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -210,14 +210,14 @@ export default class AccessListEIP2930Transaction extends BaseTransaction<Access
* The amount of gas paid for the data in this tx
*/
getDataFee(): BN {
if (Object.isFrozen(this) && this.cache.dataFee) {
if (this.cache.dataFee) {
return this.cache.dataFee
}

const cost = super.getDataFee()
cost.iaddn(AccessLists.getDataFeeEIP2930(this.accessList, this.common))

if (Object.isFrozen(this) && !this.cache.dataFee) {
if (Object.isFrozen(this)) {
this.cache.dataFee = cost
}

Expand Down
15 changes: 11 additions & 4 deletions packages/tx/src/legacyTransaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,13 @@ export default class Transaction extends BaseTransaction<Transaction> {
}

const freeze = opts?.freeze ?? true

if (freeze) {
Object.freeze(this)

this.common.on('hardforkChanged', () => {
delete this.cache.dataFee
})
}
}

Expand Down Expand Up @@ -222,12 +227,14 @@ export default class Transaction extends BaseTransaction<Transaction> {
* The amount of gas paid for the data in this tx
*/
getDataFee(): BN {
if (Object.isFrozen(this)) {
if (!this.cache.dataFee) {
this.cache.dataFee = super.getDataFee()
}
if (this.cache.dataFee) {
return this.cache.dataFee
}

if (Object.isFrozen(this)) {
this.cache.dataFee = super.getDataFee()
}

return super.getDataFee()
}

Expand Down
11 changes: 11 additions & 0 deletions packages/tx/test/legacy.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,17 @@ tape('[Transaction]', function (t) {
st.end()
})

t.test('getDataFee() -> should invalidate cached value on hardfork change', function (st) {
const common = new Common({ chain: Chain.Mainnet, hardfork: Hardfork.Byzantium })
const tx = Transaction.fromValuesArray(txFixtures[0].raw.map(toBuffer), {
common,
})
st.equals(tx.getDataFee().toNumber(), 656)
tx.common.setHardfork(Hardfork.Istanbul)
st.equals(tx.getDataFee().toNumber(), 240)
st.end()
})

t.test('getUpfrontCost() -> should return upfront cost', function (st) {
const tx = Transaction.fromTxData({
gasPrice: 1000,
Expand Down

1 comment on commit abcfd9c

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: abcfd9c Previous: 85b4253 Ratio
Block 9422913 7408 ops/sec (±17.13%) 18352 ops/sec (±9.23%) 2.48

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.