Skip to content

Remove payerSwap from transaction sorting by price#525

Merged
tungng98 merged 6 commits intoBuildOnViction:pre-releasefrom
c98tristan:fix/sort-tx-by-price
Jun 9, 2025
Merged

Remove payerSwap from transaction sorting by price#525
tungng98 merged 6 commits intoBuildOnViction:pre-releasefrom
c98tristan:fix/sort-tx-by-price

Conversation

@c98tristan
Copy link
Copy Markdown
Contributor

Problem

The transactions should be sort by gas price but if the transaction is VRC25 tx the gas price will be treated as the same price (0.25 Gwei).

Introduction

This pull request refactors the TxByPrice sorting mechanism by simplifying its structure and removing special handling for TRC21 transactions. It also updates the corresponding test cases to ensure correctness and stability of the changes. The most important changes include the removal of payersSwap from the TxByPrice struct, updates to the sorting logic, and the addition of comprehensive test cases for transaction sorting.

Refactoring and Simplification:

  • Simplified TxByPrice structure: The payersSwap field was removed, and the TxByPrice type was redefined as an alias for Transactions, eliminating special handling for TRC21 transactions. This simplifies the sorting logic to rely solely on gas prices. (core/types/transaction.go) [1] [2]
  • Updated sorting logic: Modifications were made to methods like Less, Push, and Pop to align with the simplified TxByPrice structure. Sorting now exclusively considers gas prices without any additional conditions. (core/types/transaction.go) [1] [2] [3]

Test Enhancements:

  • New test cases for sorting: Added multiple test cases to validate the new sorting behavior, including scenarios for stable sorting, heap operations, and handling of transactions with equal gas prices. (core/types/transaction_test.go)
  • Removed TRC21-specific test logic: Updated existing tests to align with the removal of TRC21-specific prioritization, ensuring all transactions are sorted purely by gas price. (core/types/transaction_test.go)

Codebase Updates:

  • Updated function signatures: Removed the payersSwap parameter from the NewTransactionsByPriceAndNonce function and updated all its invocations accordingly. (core/types/transaction.go, miner/worker.go) [1] [2] [3]
  • Import adjustments: Added necessary imports like container/heap and sort to support the new test cases. (core/types/transaction_test.go)

@@ -320,7 +320,7 @@ func (self *worker) update() {
acc, _ := types.Sender(self.current.signer, ev.Tx)
txs := map[common.Address]types.Transactions{acc: {ev.Tx}}
feeCapacity := state.GetTRC21FeeCapacityFromState(self.current.state)
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 remove feeCapacity to improve worker performance ?

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.

No, it's used in another function below.

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.

you mean commitTransactions?

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.

got it

@tungng98
Copy link
Copy Markdown
Collaborator

tungng98 commented Jun 4, 2025

Can you explain what PayerSwap does?

@c98tristan
Copy link
Copy Markdown
Contributor Author

Can you explain what PayerSwap does?

payersSwap is a map from VRC25/TRC21 contract address to its fee capacity. It's passed into the NewTransactionsByPriceAndNonce function and assigned to the transaction sorting heap. It'll validate the transaction is agas sponsor transaction, then overrides the actual gas price of the transaction to common.TRC21GasPrice.

@imduchuyyy
Copy link
Copy Markdown
Contributor

Will an older transaction (TomoZ) be replaced by a new transaction with a higher gas price?

@tungng98 tungng98 merged commit d432d3c into BuildOnViction:pre-release Jun 9, 2025
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.

3 participants