driver: get access list for balance calculation #3300
Conversation
| @@ -70,6 +74,26 @@ struct Inner { | |||
| current_block: CurrentBlockWatcher, | |||
| } | |||
|
|
|||
| pub struct Tx { | |||
There was a problem hiding this comment.
I'm not convinced we need this intermediary type.
There was a problem hiding this comment.
If we don't have this type, then from the tx builder we would need to unwrap before calling the function just so in the function it is done a: Some(a). I am open to suggestions!
There was a problem hiding this comment.
Seems like unwrapping would be the better choice than introducing more identically named types.
There was a problem hiding this comment.
In the end, unwrapping is not an option because it is not the same to be None and to be the default value. I left this struct, but I am open to other solution like creating a new function for creating access list for the balance call.
|
If this is still WIP, please convert it to draft. |
technically it is not WIP, I received comments which need to be reworked 🤔 but the more eyes and reviews I get the better. |
| @@ -70,6 +74,26 @@ struct Inner { | |||
| current_block: CurrentBlockWatcher, | |||
| } | |||
|
|
|||
| pub struct Tx { | |||
There was a problem hiding this comment.
Seems like unwrapping would be the better choice than introducing more identically named types.
| method.tx.data.clone().unwrap(), | ||
| ); | ||
| // Create the access list for the balance simulation | ||
| let access_list_tx = Tx { |
There was a problem hiding this comment.
I tested it and it returns Some(AccessList {...})
| let access_list_tx = Tx { | ||
| from: access_list_call.from.unwrap_or_default(), | ||
| to: access_list_call.to, | ||
| value: Some(0.into()), |
There was a problem hiding this comment.
This being None (which seems to be always None) or 0 does not affect the result.
Description
From the task:
This order was filled sub-optimally in 2 partial fills.
It contains a pre-hook that claims rewards to provide the full sell token balance to the owner. The driver already computes the available balances for an order based on the outcome of the pre-hooks (if there are any). But in this case the balance simulation didn't show that the owner would get all the necessary sell tokens from the hook so it reported the available balance to be whatever the owner currently has without the hook.
A solver picked up this order and executed it. In the actual settlement the hook got executed and unlocked the remaining sell tokens which led to another partial fill that then fulfilled the order completely.
When simulating the hook on its own it exceeds its gas limit and therefor reverts. This explains why no balance change happens in the simulation. However, in the actual settlement the balance change happened.
When comparing the hook simulation with the onchain settlement simulation it became apparent that executing the hook as part of the actual settlement was a lot cheaper.
The reason seems to be that the storage loads are a lot cheaper in the real settlement. That is likely caused by the fact that we use access lists for the real solution but not during the balance simulation.
Changes
How to test
Related Issues
Fixes #3252