Introduce Component for Balance Overrides

[nlordell]

Description

This PR is the first in a stack to add a system for overriding balances so that more quotes can be verified.

One limitation with the current quote verification system, is that it requires that the from account in the quote has the sell token balance available (or available after a pre-hook is executed) in order for the quote to be properly verified. This isn't always possible for all flows (and notably flows at Safe, where transactions to prepare the balance happens at the same time as the setPreSignature transaction executes, so after the quote).

The overall solution I would propose (hopefully a pragmatic one that isn't considered too hacky) would be enable special handling for the most commonly traded tokens, by configuring for each token how the storage slot is computed for the token balance. This way, you could maintain a file that contains a token => computation_strategy map for the most popular tokens allowing trades to be verified even for quotes from users without the balance available.

This PR is the first piece for this overall solution, which introduces a component for computing storage slots needed for overriding balances for eth_call simulations. If this strategy is accepted, in a follow up PRs I would:

1. Add the component to the trade verifier and use it to fund the trader when simulating quotes (I have an idea on how to do this: you would override the balance of the Solver simulation entrypoint, which would top up the Trader balance if needed; this way the missing balance can be reported as part of the simulation and logged).
2. Pipe configuration to the trade verifier and balance overrides component

Changes

• Introduces a new BalanceOverriding component

How to test

Added unit tests verifying logic.

[github-actions]

This pull request has been marked as stale because it has been inactive a while. Please update this pull request or it will be automatically closed.

[fleupold]

@MartinquaXD is on vacation and will be back mid next week to take a look

[nlordell]

Note that I finished the follow up PR with a working E2E test 🎉. I created the PR to my fork (since I don't know how to create PR stacks when forking a repo).

nlordell#1

[github-actions]

This pull request has been marked as stale because it has been inactive a while. Please update this pull request or it will be automatically closed.

[nlordell]

Please don't close! I humbly request @MartinquaXD to save my PR from the Mark and Sweep Garbage Collector.

[MartinquaXD]

I think the idea is pretty cool and can be very powerful but I'm a bit worried about the overhead of maintaining the override list. Do you have an idea how we could automatically detect which storage slot needs to be overwritten?
I would imagine a multi step approach for these overrides:
0. check if trader has the required balance and do regular simulation in that case

1. if we need to fake the balance check configuration file with hardcoded override slots
2. if 1 has no entry try to automatically detect the necessary storage slot (cache it to improve latency, in case that changes over time we can evict old entries from that cache)
3. simulate with the faked balance

That way we can make use of that feature with minimal intervention and maintenance overhead. Ideally we'd only use the hardcoded list for relevant tokens with deterministic target slots which we can't automatically detect for whatever reason.

WDYT?

Edit: Maybe we can use eth_createAccessList with a balanceOf() call and apply some heuristics? Hopefully for the majority of reasonable tokens the list should be extremely short. 🤔

[nlordell]

I'm a bit worried about the overhead of maintaining the override list.

Agree that this is kind of annoying.

2. if 1 has no entry try to automatically detect the necessary storage slot

This is possible, and I have a decent method for doing it and have played around with some ideas. I will implement it on top of my other PR in nlordell#1.

Just wanted to get a rough sense of "is this something that can eventually be merged into the codebase" before going too deep into the rabbit hole :P

[MartinquaXD]

Just wanted to get a rough sense of "is this something that can eventually be merged into the codebase" before going too deep into the rabbit hole :P

We would definitely be happy about more verifiable quotes. We just need to find a good balance between manual overhead, correctness and complexity. I think we are on a good path. 👍

[nlordell]

Edit: Maybe we can use eth_createAccessList with a balanceOf() call and apply some heuristics? Hopefully for the majority of reasonable tokens the list should be extremely short. 🤔

My idea was even sillier, but slightly more straight forward and only relies on storage overrides, which you need for simulation anyway (while eth_createAccessList isn't always available AFAIU):

1. compute an address from a hash with no known pre-image (so it can't collide with any special addresses the contract may be configured for)
2. Compute, for each storage slot from 0..N a state override with a unique value
3. Do an eth_call with the state overrides to balanceOf and match the returned balance with one of the unique values
   • If the balance matches a unique value - bingo!
   • Else, you can't provide a state override

[nlordell]

@MartinquaXD - regarding auto-detection I implemented something here: nlordell#3

I will make the final PR change the balance_override component to make use of this new component, at which point I think the PR stack is ready for proper review.

[nlordell]

Alright @MartinquaXD, I think the PR stack is ready and freshly rebased to latest main.

• This PR: Kicks things off by adding the balance overriding component (despite not using it yet).
• Simulated Token Balances for Verified Quotes nlordell/cowprotocol-services#1: Implements mocking token balances for quote verification for configured tokens
• Use Separate Account for Prefunding Trader nlordell/cowprotocol-services#2: Moves funding to a separate account in order to not interfere with the settlement process
• Add A Balance Override Detector nlordell/cowprotocol-services#3: Implements an auto-detection strategy to not rely on hard-coded configurations
• Enable Auto-detection of Token Balance Overrides nlordell/cowprotocol-services#4: Modifies the BalanceOverriding implementation to support both hard-coded pre-configured token settings, as well as use the auto-detecting mechanism introduced right above

I'm not sure what the best way to review such a stack would be. I made sure to keep my commits nice and tidy (1 commit per PR; and each commit is self-contained and can be merged into main) so we can either:

1. Do one PR at a time; when the previous PR in the stack merges, I move the PR from my repository here
2. Create a big PR where each commit should be reviewed individually

Let me know what you prefer and makes your lives easier :)

[MartinquaXD]

I think it's okay to merge the PRs individually since I already have a good understanding of the whole stack.

[nlordell]

Required statuses must pass before merging

looks like you need to "approve and run" CI.

[nlordell]

Hmm, looks like forked node connection issues:

 2024-12-05T11:32:13.738Z  WARN observe::tracing_reload_handler: open log filter reload socket file="/tmp/log_filter_override_e2e-35cee55d022e1dd4_2952.sock"
2024-12-05T11:32:13.778Z ERROR observe::tracing: thread 'banned_users::forked_node_mainnet_single_limit_order' panicked at /home/runner/work/services/services/crates/e2e/src/nodes/mod.rs:88:14:
called `Result::unwrap()` on an `Err` value: RecvError(())

Not sure how to debug this TBH.

[MartinquaXD]

Hmm, looks like forked node connection issues
Not sure how to debug this TBH.

This is a known issue. Github doesn't pass the node url secret to PRs coming from forked repos. So all is good with this PR.

[sunce86]

Since oncall and tight with time for reviewing, I just read the description and reviewed this PR, so far LGTM.