-
Notifications
You must be signed in to change notification settings - Fork 208
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should take as args simulated_order,previous_orders since it's always called in match SimulatedResult::Success no need to recheck that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Never returns error!!! Remove Result
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's cancelling it's own order?
It should only cancell the order returned by mark_mempool_unlock.
If it wants to express that the order analized should not be inserted it should return something like a bool saying it so we don’t send and order and it’s cancellation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will return for a user tx having a previous nonce dep, are we ok with this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is NO guaranty that a AceUnlockSource::User will be executed before orders depending o it.
A depending order could make a lot of money so it could be placed before the AceUnlockSource::User.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be fine. otherwise we are making a lot more downstream changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nobody uses the Vec in the result. Remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why simulate_all_orders_with_sim_tree is calling "sim_tree.add_ace_dependency_for_order" manually? Isn't submit_simulation_tasks_results doing that already?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here parents contains only Nonce dependencies and not any other prev ACE dependency.
If the order was depending in another ACE dependency that will not be included.
If looks like you should add all deps in PendingOrder::ace_unlock_contracts not only this one last unlocked.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this commet just be "ACE contract needed to be unlocked"?
You can have this pending without alread having an unlock parent.....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are not adding Nonce deps here.
ZanCorDX
commented
Jan 9, 2026
From the comment I made in add_ace_dependency_for_order and process_simulation_task_result it feels that you need a common func that computes the parent.
Maybe get_order_dependency_state gets renamed to get_order_nonce_dependency_state (since it only computes nonce deps and the new function get_order_dependency_state takes an extra parameter with the ACE needed contracts and also calls get_order_dependency_state ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All txts are being iterated, even if they where droped
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sim_success is for the whole Order, it's not related to the particular execution of this Tx.
This might be a problem since we don't have at the moment trace state per child tx.
Eg: For a Bundle we could have a Dropeable/Revertable Tx accessing the pool and failing but the bundle succeding so we would have classify_ace_interaction answering Unlocking.
To detect Force/Optional unlocking orders we could asume that Order must be a single Tx.
; Conflicts: ; crates/rbuilder/src/backtest/execute.rs ; crates/rbuilder/src/bin/run-bundle-on-prefix.rs ; crates/rbuilder/src/building/block_orders/order_priority.rs ; crates/rbuilder/src/building/block_orders/test_data_generator.rs ; crates/rbuilder/src/building/builders/parallel_builder/block_building_result_assembler.rs ; crates/rbuilder/src/building/builders/parallel_builder/conflict_resolvers.rs ; crates/rbuilder/src/building/builders/parallel_builder/conflict_task_generator.rs ; crates/rbuilder/src/building/builders/parallel_builder/groups.rs ; crates/rbuilder/src/building/sim.rs ; crates/rbuilder/src/building/testing/bundle_tests/setup.rs ; crates/rbuilder/src/live_builder/base_config.rs ; crates/rbuilder/src/live_builder/building/mod.rs ; crates/rbuilder/src/live_builder/mod.rs
...ncy providers The Entry::Occupied branch in SimTree::process_simulation_task_result compared dependency providers by coinbase_profit for all dep types, allowing a more profitable optional unlock to displace a force unlock. Now AceUnlock deps compare by interaction priority (Force > Optional > User) while Nonce deps retain profit-based comparison. Co-Authored-By: Frankenstein <263438330+frankenstien-dev@users.noreply.github.com>
When simulating an order with ACE unlock parents, the tracer accumulated
state traces from both parent and child execution. This caused the child
order to be misclassified as Unlocking { source: User } when it merely
reads the unlocked detection slot, because parent writes leaked into the
combined trace used for classification.
Adds a has_ace_parents flag to SimulationRequest that, when set, drains
the accumulated parent traces before executing the target order so ACE
interaction classification reflects only the order's own slot access.
; Conflicts: ; crates/rbuilder/src/building/sim.rs ; crates/rbuilder/src/live_builder/simulation/sim_worker.rs ; crates/rbuilder/src/live_builder/simulation/simulation_job.rs
Only register nonce dependencies as providers when a simulation updates exactly one nonce, matching develop's sims_that_update_one_nonce behavior. Multi-nonce sims (e.g. bundles with multiple senders) are stored but not tracked as providers. ACE unlock deps are always processed regardless.
Uh oh!
There was an error while loading. Please reload this page.
📝 Summary
Adjusted the ingestion pipeline to allow for specific interactions with known ace protocols to modify the simulation process to allow orders, in which we know are valid with a defined ace protocol through. Also some minor changes to the block building process putting protocol ace tx's that don't interact with state at the top
💡 Motivation and Context
Currently Other builders have started to include ace protocols and we want to also have buildernet cover this. Ace protocols have specific sequencing rules enforced on-chain that allow them to dictate execution order. By allowing for ace protocols to integrate into builders, It unlocks more transactions to execute in the same block that wouldn't be able to otherwise. The increase in transaction flow should help the builder stay competitive.
Questions
This is my first time in the codebase. I wanted to open up this PR so I could get some feedback to ensure that with the given architecture that everything is implemented in the right locations. I will add a bunch of tests once I know that the architecture is solid, just don't want to do a bunch more work when I'm uncertain about design.
✅ I have completed the following steps:
make lintmake test