core/chains/evm/client: remove copy of eth Transaction with atomic fields #7750

jmank88 · 2022-10-25T13:35:21Z

This defensive copy is not necessary (the type is safe for concurrent use and has internal atomic caches) and actually creates problems because it is invalid to copy sync/atomic values around like this.

==================
WARNING: DATA RACE
Write at 0x00c00157a6e8 by goroutine 388:
  sync/atomic.CompareAndSwapInt64()
      /opt/hostedtoolcache/go/1.19.2/x64/src/runtime/race_amd64.s:316 +0xb
  sync/atomic.CompareAndSwapPointer()
      /opt/hostedtoolcache/go/1.19.2/x64/src/runtime/atomic_pointer.go:76 +0x5e
  github.com/ethereum/go-ethereum/core/types.(*Transaction).Hash()
      /home/runner/go/pkg/mod/github.com/ethereum/go-ethereum@v1.10.25/core/types/transaction.go:371 +0x28b
  github.com/ethereum/go-ethereum/core/types.(*Transaction).MarshalJSON()
      /home/runner/go/pkg/mod/github.com/ethereum/go-ethereum@v1.10.25/core/types/transaction_marshalling.go:57 +0xb5
  encoding/json.marshalerEncoder()
      /opt/hostedtoolcache/go/1.19.2/x64/src/encoding/json/encode.go:478 +0x132
  encoding/json.(*encodeState).reflectValue()
      /opt/hostedtoolcache/go/1.19.2/x64/src/encoding/json/encode.go:359 +0x88
  encoding/json.(*encodeState).marshal()
      /opt/hostedtoolcache/go/1.19.2/x64/src/encoding/json/encode.go:331 +0x20b
  encoding/json.(*Encoder).Encode()
      /opt/hostedtoolcache/go/1.19.2/x64/src/encoding/json/stream.go:206 +0xcf
  go.uber.org/zap/zapcore.(*jsonEncoder).encodeReflected()
      /home/runner/go/pkg/mod/go.uber.org/zap@v1.23.0/zapcore/json_encoder.go:176 +0x96
  go.uber.org/zap/zapcore.(*jsonEncoder).AddReflected()
      /home/runner/go/pkg/mod/go.uber.org/zap@v1.23.0/zapcore/json_encoder.go:184 +0x64
  go.uber.org/zap/zapcore.Field.AddTo()
      /home/runner/go/pkg/mod/go.uber.org/zap@v1.23.0/zapcore/field.go:170 +0xa6c
  go.uber.org/zap/zapcore.addFields()
      /home/runner/go/pkg/mod/go.uber.org/zap@v1.23.0/zapcore/field.go:210 +0x1c4
  go.uber.org/zap/zapcore.consoleEncoder.writeContext()
      /home/runner/go/pkg/mod/go.uber.org/zap@v1.23.0/zapcore/console_encoder.go:141 +0xbe
  go.uber.org/zap/zapcore.consoleEncoder.EncodeEntry()
      /home/runner/go/pkg/mod/go.uber.org/zap@v1.23.0/zapcore/console_encoder.go:119 +0xc49
  go.uber.org/zap/zapcore.(*ioCore).Write()
      /home/runner/go/pkg/mod/go.uber.org/zap@v1.23.0/zapcore/core.go:95 +0x10c
  go.uber.org/zap/zapcore.(*CheckedEntry).Write()
      /home/runner/go/pkg/mod/go.uber.org/zap@v1.23.0/zapcore/entry.go:255 +0x2ce
  go.uber.org/zap.(*SugaredLogger).log()
      /home/runner/go/pkg/mod/go.uber.org/zap@v1.23.0/sugar.go:287 +0x13a
  go.uber.org/zap.(*SugaredLogger).Debugw()
      /home/runner/go/pkg/mod/go.uber.org/zap@v1.23.0/sugar.go:196 +0xab
  github.com/smartcontractkit/chainlink/core/logger.(*zapLogger).Debugw()
      <autogenerated>:1 +0x29
  github.com/smartcontractkit/chainlink/core/chains/evm/client.(*Pool).SendTransaction.func1.1()
      /home/runner/work/chainlink/chainlink/core/chains/evm/client/pool.go:323 +0x416
  github.com/smartcontractkit/chainlink/core/chains/evm/client.(*Pool).SendTransaction.func1.2()
      /home/runner/work/chainlink/chainlink/core/chains/evm/client/pool.go:331 +0xe1

Previous read at 0x00c00157a6e8 by goroutine 141:
  github.com/smartcontractkit/chainlink/core/chains/evm/client.(*Pool).SendTransaction.func1()
      /home/runner/work/chainlink/chainlink/core/chains/evm/client/pool.go:331 +0x135
  github.com/smartcontractkit/chainlink/core/utils.(*StartStopOnce).IfNotStopped()
      /home/runner/work/chainlink/chainlink/core/utils/utils.go:958 +0xcf
  github.com/smartcontractkit/chainlink/core/chains/evm/client.(*Pool).SendTransaction()
      /home/runner/work/chainlink/chainlink/core/chains/evm/client/pool.go:313 +0x2cc
  github.com/smartcontractkit/chainlink/core/chains/evm/client.(*client).SendTransaction()
      /home/runner/work/chainlink/chainlink/core/chains/evm/client/client.go:207 +0x65
  github.com/smartcontractkit/chainlink/core/chains/evm/client_test.TestEthClient_SendTransaction_WithSecondaryURLs()
      /home/runner/work/chainlink/chainlink/core/chains/evm/client/client_test.go:324 +0x7fb
  testing.tRunner()
      /opt/hostedtoolcache/go/1.19.2/x64/src/testing/testing.go:1446 +0x216
  testing.(*T).Run.func1()
      /opt/hostedtoolcache/go/1.19.2/x64/src/testing/testing.go:1493 +0x47

Goroutine 388 (running) created at:
  github.com/smartcontractkit/chainlink/core/chains/evm/client.(*Pool).SendTransaction.func1()
      /home/runner/work/chainlink/chainlink/core/chains/evm/client/pool.go:316 +0x274
  github.com/smartcontractkit/chainlink/core/utils.(*StartStopOnce).IfNotStopped()
      /home/runner/work/chainlink/chainlink/core/utils/utils.go:958 +0xcf
  github.com/smartcontractkit/chainlink/core/chains/evm/client.(*Pool).SendTransaction()
      /home/runner/work/chainlink/chainlink/core/chains/evm/client/pool.go:313 +0x2cc
  github.com/smartcontractkit/chainlink/core/chains/evm/client.(*client).SendTransaction()
      /home/runner/work/chainlink/chainlink/core/chains/evm/client/client.go:207 +0x65
  github.com/smartcontractkit/chainlink/core/chains/evm/client_test.TestEthClient_SendTransaction_WithSecondaryURLs()
      /home/runner/work/chainlink/chainlink/core/chains/evm/client/client_test.go:324 +0x7fb
  testing.tRunner()
      /opt/hostedtoolcache/go/1.19.2/x64/src/testing/testing.go:1446 +0x216
  testing.(*T).Run.func1()
      /opt/hostedtoolcache/go/1.19.2/x64/src/testing/testing.go:1493 +0x47

Goroutine 141 (running) created at:
  testing.(*T).Run()
      /opt/hostedtoolcache/go/1.19.2/x64/src/testing/testing.go:1493 +0x75d
  testing.runTests.func1()
      /opt/hostedtoolcache/go/1.19.2/x64/src/testing/testing.go:1846 +0x99
  testing.tRunner()
      /opt/hostedtoolcache/go/1.19.2/x64/src/testing/testing.go:1446 +0x216
  testing.runTests()
      /opt/hostedtoolcache/go/1.19.2/x64/src/testing/testing.go:1844 +0x7ec
  testing.(*M).Run()
      /opt/hostedtoolcache/go/1.19.2/x64/src/testing/testing.go:1726 +0xa84
  main.main()
      _testmain.go:109 +0x2e9
==================
==================
WARNING: DATA RACE
Write at 0x00c00157a6f0 by goroutine 388:
  sync/atomic.StoreInt64()
      /opt/hostedtoolcache/go/1.19.2/x64/src/runtime/race_amd64.s:237 +0xb
  sync/atomic.StorePointer()
      /opt/hostedtoolcache/go/1.19.2/x64/src/runtime/atomic_pointer.go:51 +0x44
  github.com/ethereum/go-ethereum/core/types.(*Transaction).Hash()
      /home/runner/go/pkg/mod/github.com/ethereum/go-ethereum@v1.10.25/core/types/transaction.go:371 +0x28b
  github.com/ethereum/go-ethereum/core/types.(*Transaction).MarshalJSON()
      /home/runner/go/pkg/mod/github.com/ethereum/go-ethereum@v1.10.25/core/types/transaction_marshalling.go:57 +0xb5
  encoding/json.marshalerEncoder()
      /opt/hostedtoolcache/go/1.19.2/x64/src/encoding/json/encode.go:478 +0x132
  encoding/json.(*encodeState).reflectValue()
      /opt/hostedtoolcache/go/1.19.2/x64/src/encoding/json/encode.go:359 +0x88
  encoding/json.(*encodeState).marshal()
      /opt/hostedtoolcache/go/1.19.2/x64/src/encoding/json/encode.go:331 +0x20b
  encoding/json.(*Encoder).Encode()
      /opt/hostedtoolcache/go/1.19.2/x64/src/encoding/json/stream.go:206 +0xcf
  go.uber.org/zap/zapcore.(*jsonEncoder).encodeReflected()
      /home/runner/go/pkg/mod/go.uber.org/zap@v1.23.0/zapcore/json_encoder.go:176 +0x96
  go.uber.org/zap/zapcore.(*jsonEncoder).AddReflected()
      /home/runner/go/pkg/mod/go.uber.org/zap@v1.23.0/zapcore/json_encoder.go:184 +0x64
  go.uber.org/zap/zapcore.Field.AddTo()
      /home/runner/go/pkg/mod/go.uber.org/zap@v1.23.0/zapcore/field.go:170 +0xa6c
  go.uber.org/zap/zapcore.addFields()
      /home/runner/go/pkg/mod/go.uber.org/zap@v1.23.0/zapcore/field.go:210 +0x1c4
  go.uber.org/zap/zapcore.consoleEncoder.writeContext()
      /home/runner/go/pkg/mod/go.uber.org/zap@v1.23.0/zapcore/console_encoder.go:141 +0xbe
  go.uber.org/zap/zapcore.consoleEncoder.EncodeEntry()
      /home/runner/go/pkg/mod/go.uber.org/zap@v1.23.0/zapcore/console_encoder.go:119 +0xc49
  go.uber.org/zap/zapcore.(*ioCore).Write()
      /home/runner/go/pkg/mod/go.uber.org/zap@v1.23.0/zapcore/core.go:95 +0x10c
  go.uber.org/zap/zapcore.(*CheckedEntry).Write()
      /home/runner/go/pkg/mod/go.uber.org/zap@v1.23.0/zapcore/entry.go:255 +0x2ce
  go.uber.org/zap.(*SugaredLogger).log()
      /home/runner/go/pkg/mod/go.uber.org/zap@v1.23.0/sugar.go:287 +0x13a
  go.uber.org/zap.(*SugaredLogger).Debugw()
      /home/runner/go/pkg/mod/go.uber.org/zap@v1.23.0/sugar.go:196 +0xab
  github.com/smartcontractkit/chainlink/core/logger.(*zapLogger).Debugw()
      <autogenerated>:1 +0x29
  github.com/smartcontractkit/chainlink/core/chains/evm/client.(*Pool).SendTransaction.func1.1()
      /home/runner/work/chainlink/chainlink/core/chains/evm/client/pool.go:323 +0x416
  github.com/smartcontractkit/chainlink/core/chains/evm/client.(*Pool).SendTransaction.func1.2()
      /home/runner/work/chainlink/chainlink/core/chains/evm/client/pool.go:331 +0xe1

Previous read at 0x00c00157a6f0 by goroutine 141:
  github.com/smartcontractkit/chainlink/core/chains/evm/client.(*Pool).SendTransaction.func1()
      /home/runner/work/chainlink/chainlink/core/chains/evm/client/pool.go:331 +0x135
  github.com/smartcontractkit/chainlink/core/utils.(*StartStopOnce).IfNotStopped()
      /home/runner/work/chainlink/chainlink/core/utils/utils.go:958 +0xcf
  github.com/smartcontractkit/chainlink/core/chains/evm/client.(*Pool).SendTransaction()
      /home/runner/work/chainlink/chainlink/core/chains/evm/client/pool.go:313 +0x2cc
  github.com/smartcontractkit/chainlink/core/chains/evm/client.(*client).SendTransaction()
      /home/runner/work/chainlink/chainlink/core/chains/evm/client/client.go:207 +0x65
  github.com/smartcontractkit/chainlink/core/chains/evm/client_test.TestEthClient_SendTransaction_WithSecondaryURLs()
      /home/runner/work/chainlink/chainlink/core/chains/evm/client/client_test.go:324 +0x7fb
  testing.tRunner()
      /opt/hostedtoolcache/go/1.19.2/x64/src/testing/testing.go:1446 +0x216
  testing.(*T).Run.func1()
      /opt/hostedtoolcache/go/1.19.2/x64/src/testing/testing.go:1493 +0x47

Goroutine 388 (running) created at:
  github.com/smartcontractkit/chainlink/core/chains/evm/client.(*Pool).SendTransaction.func1()
      /home/runner/work/chainlink/chainlink/core/chains/evm/client/pool.go:316 +0x274
  github.com/smartcontractkit/chainlink/core/utils.(*StartStopOnce).IfNotStopped()
      /home/runner/work/chainlink/chainlink/core/utils/utils.go:958 +0xcf
  github.com/smartcontractkit/chainlink/core/chains/evm/client.(*Pool).SendTransaction()
      /home/runner/work/chainlink/chainlink/core/chains/evm/client/pool.go:313 +0x2cc
  github.com/smartcontractkit/chainlink/core/chains/evm/client.(*client).SendTransaction()
      /home/runner/work/chainlink/chainlink/core/chains/evm/client/client.go:207 +0x65
  github.com/smartcontractkit/chainlink/core/chains/evm/client_test.TestEthClient_SendTransaction_WithSecondaryURLs()
      /home/runner/work/chainlink/chainlink/core/chains/evm/client/client_test.go:324 +0x7fb
  testing.tRunner()
      /opt/hostedtoolcache/go/1.19.2/x64/src/testing/testing.go:1446 +0x216
  testing.(*T).Run.func1()
      /opt/hostedtoolcache/go/1.19.2/x64/src/testing/testing.go:1493 +0x47

Goroutine 141 (running) created at:
  testing.(*T).Run()
      /opt/hostedtoolcache/go/1.19.2/x64/src/testing/testing.go:1493 +0x75d
  testing.runTests.func1()
      /opt/hostedtoolcache/go/1.19.2/x64/src/testing/testing.go:1846 +0x99
  testing.tRunner()
      /opt/hostedtoolcache/go/1.19.2/x64/src/testing/testing.go:1446 +0x216
  testing.runTests()
      /opt/hostedtoolcache/go/1.19.2/x64/src/testing/testing.go:1844 +0x7ec
  testing.(*M).Run()
      /opt/hostedtoolcache/go/1.19.2/x64/src/testing/testing.go:1726 +0xa84
  main.main()
      _testmain.go:109 +0x2e9
==================

…elds

github-actions · 2022-10-25T14:59:32Z

I see that you haven't updated any CHANGELOG files. Would it make sense to do so?

github-actions · 2022-10-25T15:15:28Z

Solana Smoke Test Results

1 tests 1 ✔️ 4m 29s ⏱️
1 suites 0 💤
1 files 0 ❌

Results for commit a533e65.

github-actions · 2022-10-25T15:22:23Z

EVM Smoke Test Results

82 tests 34 ✔️ 8m 8s ⏱️
  1 suites 48 💤
  1 files   0 ❌

Results for commit a533e65.

core/chains/evm/client: remove copy of eth Transaction with atomic fi…

90bc5ad

…elds

jmank88 marked this pull request as ready for review October 25, 2022 13:41

jmank88 requested a review from samsondav as a code owner October 25, 2022 13:41

jmank88 requested review from vyzaldysanchez, reductionista, pinebit, bolekk, essamhassan and dimriou October 25, 2022 13:50

samsondav approved these changes Oct 25, 2022

View reviewed changes

Merge branch 'develop' into tx-copy-race

a533e65

jmank88 merged commit 20b44bc into develop Oct 25, 2022

jmank88 deleted the tx-copy-race branch October 25, 2022 15:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

core/chains/evm/client: remove copy of eth Transaction with atomic fields #7750

core/chains/evm/client: remove copy of eth Transaction with atomic fields #7750

jmank88 commented Oct 25, 2022

github-actions bot commented Oct 25, 2022

github-actions bot commented Oct 25, 2022

github-actions bot commented Oct 25, 2022

core/chains/evm/client: remove copy of eth Transaction with atomic fields #7750

core/chains/evm/client: remove copy of eth Transaction with atomic fields #7750

Conversation

jmank88 commented Oct 25, 2022

github-actions bot commented Oct 25, 2022

github-actions bot commented Oct 25, 2022

Solana Smoke Test Results

github-actions bot commented Oct 25, 2022

EVM Smoke Test Results