Skip to content

remove duplicated txs#142

Merged
BeniaminDrasovean merged 4 commits into
rc/supernovafrom
remove-duplicated-txs
Mar 26, 2026
Merged

remove duplicated txs#142
BeniaminDrasovean merged 4 commits into
rc/supernovafrom
remove-duplicated-txs

Conversation

@BeniaminDrasovean

@BeniaminDrasovean BeniaminDrasovean commented Mar 19, 2026

Copy link
Copy Markdown
Contributor

This pull request refactors the logic for collecting transactions from the pool to handle duplicate transactions more robustly. The main improvement is ensuring that transactions present in both the valid and invalid pools are only processed as invalid, preventing duplicate processing. The code now uses a map to deduplicate transactions before assembling the ordered list.

Handling of duplicate transactions:

  • Changed the getTxsWithOrder function to use a map (txsWithOrderMap) instead of a slice to collect transactions, ensuring each transaction hash appears only once and is processed as invalid if present in both pools.
  • Assembles the final ordered slice from the map, maintaining correct execution order and transaction type.

Comment on lines +180 to +182
// There can be a case when a transaction is included in the block, but also marked as invalid, so it will be present
// in both transactions and invalidTxs maps from transactions pool, with the same execution order. In that case,
// we want to make sure that we process that transaction only as invalid.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we have also a unit test to cover this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added.

@AdoAdoAdo AdoAdoAdo Mar 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we need to check why the outport driver adds the transaction in two pools, I don't think this is correct.

ssd04
ssd04 previously approved these changes Mar 19, 2026

Copilot AI left a comment

Copy link
Copy Markdown

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 refactors how transaction hashes are collected and ordered from the outport.TransactionPool to avoid processing duplicate transactions twice (especially when a tx appears both as valid and invalid), and adds a test scaffold for the new behavior.

Changes:

  • Refactors getTxsWithOrder to deduplicate by tx hash using a map, ensuring invalid entries override valid ones.
  • Adds a test-only exported wrapper for getTxsWithOrder.
  • Adds a unit test that exercises a duplicate tx scenario (via a length assertion).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
process/eventsInterceptor.go Switches ordered-tx collection to a map-based dedup flow before sorting.
process/export_test.go Exposes a test-only wrapper to call getTxsWithOrder from external tests.
process/eventsInterceptor_test.go Adds a new test case for duplicate tx handling in the pool.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread process/eventsInterceptor.go
Comment thread process/eventsInterceptor_test.go Outdated
Comment thread process/eventsInterceptor_test.go Outdated
Comment thread process/export_test.go
ssd04
ssd04 previously approved these changes Mar 23, 2026
Comment thread process/export_test.go Outdated
return baseNilEventsDataChecks(eventsData)
}

type txsWithOrderExportedFields struct {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

*optional: export fields of main struct and just export it here, instead of creating separate duplicated struct for testing

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Changed

@BeniaminDrasovean BeniaminDrasovean merged commit 2ea594e into rc/supernova Mar 26, 2026
2 checks passed
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.

4 participants