Skip to content

Commit 42c5a3a

Browse files
committed
Bad blocks fix (fix datarace in ActivePrecompiles) (#312)
ActivePrecompiles referenced a global variable named PrecompiledAddressesCel2 from multiple goroutines without any synchronisation. This PR fixes that by making PrecompiledAddressesCel2 a local variable within ActivePrecompiles. This also looks like it is a fix for the bad blocks we had been seeing by causing the ActivePrecompiles return value to lack some of our active precompiles, which in turn would lead to them not being pre-warmed in the access list, which would result in them costing an extra 2500 gas when executed for the first time in a transaction. (Thank you to @Kourin1996 & @karlb for figuring this out!) See the data race trace below. op-geth-1 | WARNING: DATA RACE op-geth-1 | Read at 0x0000040a7920 by goroutine 651651: op-geth-1 | github.com/ethereum/go-ethereum/core/vm.ActivePrecompiles() op-geth-1 | github.com/ethereum/go-ethereum/core/vm/contracts.go:257 +0x35c op-geth-1 | github.com/ethereum/go-ethereum/core.(*StateTransition).innerTransitionDb() op-geth-1 | github.com/ethereum/go-ethereum/core/state_transition.go:600 +0x190 op-geth-1 | github.com/ethereum/go-ethereum/core.(*StateTransition).TransitionDb() op-geth-1 | github.com/ethereum/go-ethereum/core/state_transition.go:550 +0x148 op-geth-1 | github.com/ethereum/go-ethereum/core.ApplyMessage() op-geth-1 | github.com/ethereum/go-ethereum/core/state_transition.go:250 +0x608 op-geth-1 | github.com/ethereum/go-ethereum/core.precacheTransaction() op-geth-1 | github.com/ethereum/go-ethereum/core/state_prefetcher.go:90 +0xac op-geth-1 | github.com/ethereum/go-ethereum/core.(*statePrefetcher).Prefetch() op-geth-1 | github.com/ethereum/go-ethereum/core/state_prefetcher.go:69 +0x640 op-geth-1 | github.com/ethereum/go-ethereum/core.(*BlockChain).insertChain.func3() op-geth-1 | github.com/ethereum/go-ethereum/core/blockchain.go:1842 +0x11c op-geth-1 | github.com/ethereum/go-ethereum/core.(*BlockChain).insertChain.gowrap2() op-geth-1 | github.com/ethereum/go-ethereum/core/blockchain.go:1848 +0x78 op-geth-1 | op-geth-1 | Previous write at 0x0000040a7920 by goroutine 651491: op-geth-1 | github.com/ethereum/go-ethereum/core/vm.ActivePrecompiles() op-geth-1 | github.com/ethereum/go-ethereum/core/vm/contracts.go:257 +0x368 op-geth-1 | github.com/ethereum/go-ethereum/core.(*StateTransition).innerTransitionDb() op-geth-1 | github.com/ethereum/go-ethereum/core/state_transition.go:600 +0x190 op-geth-1 | github.com/ethereum/go-ethereum/core.(*StateTransition).TransitionDb() op-geth-1 | github.com/ethereum/go-ethereum/core/state_transition.go:550 +0x148 op-geth-1 | github.com/ethereum/go-ethereum/core.ApplyMessage() op-geth-1 | github.com/ethereum/go-ethereum/core/state_transition.go:250 +0x608 op-geth-1 | github.com/ethereum/go-ethereum/core.ApplyTransactionWithEVM() op-geth-1 | github.com/ethereum/go-ethereum/core/state_processor.go:132 +0x4e8 op-geth-1 | github.com/ethereum/go-ethereum/core.(*StateProcessor).Process() op-geth-1 | github.com/ethereum/go-ethereum/core/state_processor.go:92 +0x9cc op-geth-1 | github.com/ethereum/go-ethereum/core.(*BlockChain).processBlock() op-geth-1 | github.com/ethereum/go-ethereum/core/blockchain.go:1934 +0x3b4 op-geth-1 | github.com/ethereum/go-ethereum/core.(*BlockChain).insertChain() op-geth-1 | github.com/ethereum/go-ethereum/core/blockchain.go:1853 +0x2cd8 op-geth-1 | github.com/ethereum/go-ethereum/core.(*BlockChain).InsertChain() op-geth-1 | github.com/ethereum/go-ethereum/core/blockchain.go:1625 +0xde0 op-geth-1 | github.com/ethereum/go-ethereum/eth/downloader.(*Downloader).importBlockResults() op-geth-1 | github.com/ethereum/go-ethereum/eth/downloader/downloader.go:771 +0x6f4 op-geth-1 | github.com/ethereum/go-ethereum/eth/downloader.(*Downloader).processFullSyncContent() op-geth-1 | github.com/ethereum/go-ethereum/eth/downloader/downloader.go:742 +0xa8 op-geth-1 | github.com/ethereum/go-ethereum/eth/downloader.(*Downloader).syncToHead.func8() op-geth-1 | github.com/ethereum/go-ethereum/eth/downloader/downloader.go:537 +0x2c op-geth-1 | github.com/ethereum/go-ethereum/eth/downloader.(*Downloader).spawnSync.func1() op-geth-1 | github.com/ethereum/go-ethereum/eth/downloader/downloader.go:549 +0x90
1 parent 21afddc commit 42c5a3a

File tree

1 file changed

+6
-5
lines changed

1 file changed

+6
-5
lines changed

core/vm/contracts.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,6 @@ var PrecompiledCeloContractsCel2 = map[common.Address]CeloPrecompiledContract{
210210
var (
211211
PrecompiledAddressesIsthmus []common.Address
212212
PrecompiledAddressesGranite []common.Address
213-
PrecompiledAddressesCel2 []common.Address
214213
PrecompiledAddressesFjord []common.Address
215214
PrecompiledAddressesPrague []common.Address
216215
PrecompiledAddressesCancun []common.Address
@@ -314,14 +313,16 @@ func ActivePrecompiles(rules params.Rules) []common.Address {
314313
return addresses
315314
}
316315

317-
PrecompiledAddressesCel2 = PrecompiledAddressesCel2[:0]
318-
PrecompiledAddressesCel2 = append(PrecompiledAddressesCel2, addresses...)
316+
// We can't hardcode the cel2 precompiles because they depend on the underlying
317+
// active optimism fork, so instead we dynamically calculate them here.
318+
precompiledAddressesCel2 := make([]common.Address, 0, len(addresses)+len(PrecompiledCeloContractsCel2))
319+
precompiledAddressesCel2 = append(precompiledAddressesCel2, addresses...)
319320

320321
for k := range PrecompiledCeloContractsCel2 {
321-
PrecompiledAddressesCel2 = append(PrecompiledAddressesCel2, k)
322+
precompiledAddressesCel2 = append(precompiledAddressesCel2, k)
322323
}
323324

324-
return PrecompiledAddressesCel2
325+
return precompiledAddressesCel2
325326
}
326327

327328
// RunPrecompiledContract runs and evaluates the output of a precompiled contract.

0 commit comments

Comments
 (0)