Conversation
| impl Bundle<OpTxEnvelope> { | ||
| /// Returns the bundle hash of the bundle. | ||
| pub fn bundle_hash(&self) -> B256 { | ||
| // SAFETY: At this point, the bundle hash is guaranteed to be initialized. |
There was a problem hiding this comment.
Safety comments are for unsafe codes, not unwraps, normally
|
|
||
| /// Validates the bundle, including signature validation of included transactions. | ||
| /// | ||
| /// This is a CPU-intensive operation. |
There was a problem hiding this comment.
Maybe worth using rayon and par_iter?
| } | ||
|
|
||
| pub fn bundle_hash(&self) -> B256 { | ||
| // SAFETY: At this point, the bundle hash is guaranteed to be initialized. |
|
|
||
| impl SimulatedBundle { | ||
| /// Creates a new unsimulated bundle from a validated bundle. | ||
| pub fn new(validated: Arc<ValidatedBundle>) -> Self { |
There was a problem hiding this comment.
It feels counter to the type that we can create an unsimulated simulated bundle, and because of that a lot of methods later have to return options...
There was a problem hiding this comment.
Fully agree, was not the greatest abstraction here. We should revisit this.
To do: revisit SimulatedBundle abstraction, because it doesn't make sense and can contain unsimulated bundles.
| self.block_number.hash(state); | ||
| self.transactions.hash(state); | ||
| // FIXME: This is actually not fully compatible with <https://docs.titanbuilder.xyz/api/eth_sendbundle#bundle-hash>, | ||
| // because they use strings for the reverting tx hashes. |
There was a problem hiding this comment.
We should address this before merging as it's pretty hidden tbh but the fix afaik should be simple enough
There was a problem hiding this comment.
The bundle hash is not going to be compatible anyway until all the fields match. Seems like a bit of a waste to first hex encode it, and then hash it, for all transactions
| /// Sends a new order to the order pool. | ||
| fn handle_new_order(&mut self, order: Order, ctx: &mut SequencerContext<Db>, senders: &SendersSpine<Db>) { | ||
| // Add the (unsimulated) order to the TOF snapshot. | ||
| // TODO(mempirate): This might cause issues because it hasn't been simulated, where do we add sim info? |
There was a problem hiding this comment.
I think you handled it, no?
|
|
||
| // Commit to State's in-memory cache (not the underlying db) | ||
| // so subsequent transactions see these state changes | ||
| // TODO(mempirate): validate that this is actually the case and we're not committing to underlying db |
There was a problem hiding this comment.
Maybe we can wrap the DB in this function to ensure this?
There was a problem hiding this comment.
I took a look at this and it seems like we shouldn't do .commit_ref(&result_and_state.state); here. It does seem to commit into the underlying db (the cache not all the way to on-disk db) which could mess up the sorting logic. I think what @Karrq suggest should work. Maybe you can add DBBundle on top of DBSorting.
There was a problem hiding this comment.
But doesn't this wrap the provided DB into a new State cache? When we call commit_ref, it seems to be called on that State cache that was just created
mempirate
force-pushed
the
jonas/feat/bundle-support
branch
from
5655742 to
8f108bd
Compare
3 participants
Closes #258