Skip to content

Non mempool profit #621

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 7 commits into from
May 19, 2025
Merged

Non mempool profit #621

merged 7 commits into from
May 19, 2025

Conversation

ZanCorDX
Copy link
Collaborator

📝 Summary

SimValue now has 2 ProfitInfo instead of the single coinbase_profit/gas_used pair so now we have the full_profit_info (previous coinbase_profit/gas_used) and the new non_mempool_profit_info which excludes mempool txs profits from bundles.
Backwards compatible.
Implementation to detect mempool tx is super simple and heavily assumes that we will always see a mempool tx before a bundle including that tx:
We add a order_input::mempool_txs_detector::ReplaceableOrderStreamSniffer in the orderflow stream that updates the MempoolTxsDetector which is used on SimValue creation to filter mempool txs.

💡 Motivation and Context

This will give more info to block building algorithms.

✅ I have completed the following steps:

  • Run make lint
  • Run make test
  • Added tests (if applicable)

@ZanCorDX ZanCorDX requested review from dvush and ferranbt as code owners May 16, 2025 16:03
Copy link
Contributor

@dvush dvush left a comment

Choose a reason for hiding this comment

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

besides 2 comments, IMO its fine

} else {
error!(
non_mempool_coinbase_profit = format_ether(non_mempool_coinbase_profit),
"Non mempool orders have always positive profit, how did we got a negative value????");
Copy link
Contributor

Choose a reason for hiding this comment

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

can you please rewrite the message to make it more boring and uniform

/// Current implementation is super simple, it just checks the tx hash against a set of hashes.
#[derive(Debug)]
pub struct MempoolTxsDetector {
mempool_txs: Mutex<HashSet<TxHash>>,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think its always better to use something that has lower chance of blocking:

DashSet > RwLock<HashSet> > Mutex<HashSet>

since we use dashmap here a lot, DashSet is a great choice (but don't forget to specify ahash as a hasher instead of default hasher)

@ZanCorDX ZanCorDX merged commit e0a7bb0 into develop May 19, 2025
3 checks passed
@ZanCorDX ZanCorDX deleted the non-mempool-profit branch May 19, 2025 14:22
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.

2 participants