Skip to content

Fix false success and racy counters in generate_eoa_transactions#215

Open
damilolaedwards wants to merge 1 commit into
ethpandaops:masterfrom
damilolaedwards:fix-generate-eoa-false-success
Open

Fix false success and racy counters in generate_eoa_transactions#215
damilolaedwards wants to merge 1 commit into
ethpandaops:masterfrom
damilolaedwards:fix-generate-eoa-false-success

Conversation

@damilolaedwards

Copy link
Copy Markdown

Problem

Two problems in the result accounting of generate_eoa_transactions.

  • The success, revert and unknown counters were plain ints incremented from per-transaction receipt callbacks, which spamoor runs in their own goroutines. Concurrent increments raced and lost updates.
  • The task only waited for those callbacks when awaitReceipt was set. But failOnReject and failOnSuccess derive the result from the same counts, and awaitReceipt is off by default. So with failOnReject: true and the default awaitReceipt: false, the verdict read zeroed counters before the receipts arrived and reported success even when every transaction reverted, silently defeating the requested check.

Fix

  • Make the three counters atomic.Int64 so callback increments are not lost.
  • Wait for the receipt callbacks whenever the result depends on their counts, that is when awaitReceipt, failOnReject or failOnSuccess is set. The condition is extracted into a helper so it can be tested.

Tests

The wait condition is covered including the failOnReject without awaitReceipt case that previously skipped the wait, and a concurrent increment test confirms the counters do not lose updates and are race free.

go build ./..., go vet, and the package tests under -race pass.

Two problems in the result accounting. The success, revert and unknown
counters were plain ints incremented from per transaction receipt
callbacks that run in their own goroutines, so concurrent increments
raced and lost updates. And the task only waited for those callbacks when
awaitReceipt was set, but failOnReject and failOnSuccess derive the result
from the same counts; with awaitReceipt off (the default) the verdict read
zeroed counters before the receipts arrived and reported success even when
every transaction reverted.

Make the counters atomic, and wait for the receipt callbacks whenever the
result depends on their counts (awaitReceipt, failOnReject or
failOnSuccess). Extract that condition into a helper so it can be tested.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant