Skip to content

chain: Fix TxPool race #262

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 14, 2025
Merged

chain: Fix TxPool race #262

merged 1 commit into from
Jun 14, 2025

Conversation

lukechampine
Copy link
Member

The txpool should never expose raw transactions in the pool without deep-copying them. This was causing a data race when a parent transaction (returned by V2TransactionSet) was later mutated by applyPoolUpdate.

Fixes #261

@Copilot Copilot AI review requested due to automatic review settings June 13, 2025 04:17
@github-project-automation github-project-automation bot moved this to In Progress in Sia Jun 13, 2025
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses a data race by ensuring transactions returned from the pool are deep-copied rather than exposing the originals.

  • Use slices.Clone for PoolTransactions to copy the slice header.
  • Apply DeepCopy() to v2 transactions in TransactionsForPartialBlock and V2TransactionSet.
  • Remove a stale TODO.
Comments suppressed due to low confidence (3)

chain/manager.go:880

  • TransactionsForPartialBlock still returns raw pool transactions without deep-copy, which can reintroduce races. Consider using txn.DeepCopy() when appending to txns.
for _, txn := range m.txpool.txns {

chain/manager.go:838

  • Add unit tests for PoolTransactions, TransactionsForPartialBlock, and V2TransactionSet that verify modifying returned transactions doesn’t affect the pool, preventing future race regressions.
func (m *Manager) PoolTransactions() []types.Transaction {

chain/manager.go:841

  • Ensure the slices package is imported (import "slices") and confirm the project’s minimum Go version is 1.21 or newer to support slices.Clone.
return slices.Clone(m.txpool.txns)

@n8maninger n8maninger merged commit b6ecc61 into master Jun 14, 2025
12 checks passed
@n8maninger n8maninger deleted the txpool-race branch June 14, 2025 04:06
@github-project-automation github-project-automation bot moved this from In Progress to Done in Sia Jun 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

updateTxnProofs data race
4 participants