Skip to content
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

Simulated Token Balances for Verified Quotes #3147

Merged

Conversation

nlordell
Copy link
Contributor

@nlordell nlordell commented Dec 5, 2024

Description

This PR is a follow up to #3125 and uses the component introduced in the aforementioned PR for setting up a simulated token balance using state overrides in order for quote verification to work even when the trader does not have sufficient balance.

Changes

The way it works is by configuring known mapping slots for the mapping(address => uint256) balances in ERC20 token contract implementations and using this to compute the slot for overriding a separate account's token balance (the Spardose), which can prefund the trader during quote simualtions. We intentionally do not override the trader's or solver's balance in order to not interfere with the settlement process:

Technically a solver could have private inventory they would like to
use for the solution instead of transfering it to the trader.

Posted by @MartinquaXD

Note that the type of the state override changed slightly. This is because it was wrong to begin with. Node implementations I tested with (Geth and Anvil) expect both the slot and the value for state overrides to be exactly 32-bytes long (so H256). I guess this feature of the state override in the ethrpc crate was not used in the past and therefore no one noticed 🤷.

  • Adds command line configuration for token balance overrides
  • Adds logic to the trades verifier to setup state overrides for take token balances for the configured tokens
  • Adds logic in the settlement simulation to try and fund the trader if they are missing balances and balance overrides are enabled
  • Fixes to the StateOverride type and serialization

Test Plan

Added an E2E test that uses the new token balance override feature in order to produce a verified quote for a trader with no balances. Note that commenting out the API arguments causes the test to fail as expected.

This PR is the combination of two original PRs mentioned in #3125:

This PR is a follow up to cowprotocol#3125 and uses the component introduced in the
aforementioned PR for setting up a simulated token balance using state
overrides in order for quote verification to work even when the trader
does not have sufficient balance.

The way it works is by configuring known mapping slots for the
`mapping(address => uint256) balances` in ERC20 token contract
implementations and using this to compute the slot for overriding a
separate account's token balance (the `Spardose`), which can prefund the
trader during quote simualtions. We intentionally do not override the
trader's or solver's balance in order to not interfere with the
settlement process:

> Technically a solver could have private inventory they would like to
> use for the solution instead of transfering it to the trader.

_Posted by @MartinquaXD_

Note that the type of the state override changed slightly. This is
because it was wrong to begin with. Node implementations I tested with
(Geth and Anvil) expect both the slot and the value for state overrides
to be exactly 32-bytes long (so `H256`). I guess this feature of the
state override in the `ethrpc` crate was not used in the past and
therefore no one noticed 🤷.

 ### Test Plan

Added an E2E test that uses the new token balance override feature in
order to produce a verified quote for a trader with no balances. Note
that commenting out the API arguments causes the test to fail as
expected.
Comment on lines +90 to +93
{
(bool success,) = receiver.call{value: 0}("");
success;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just to silence a Solidity compiler warning.

Copy link
Contributor

@sunce86 sunce86 left a comment

Choose a reason for hiding this comment

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

Not too confident in my approval since I don't have a full context of quotes verification as Martin, but didn't notice anything unusual.

@MartinquaXD MartinquaXD merged commit e75a277 into cowprotocol:main Dec 5, 2024
10 of 11 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Dec 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants