Use Separate Account for Prefunding Trader #2
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nlordell
force-pushed
the
feat/token-state-override-2
branch
from
529d019 to
67fc567
Compare
nlordell
force-pushed
the
feat/token-state-override-3
branch
from
723758d to
83ae3bb
Compare
This PR builds on #1 and uses a separate account for pre-funding the trader with sell tokens in case they don't have a sufficient balance. This was done to interfere less with the settlement process: > Also technically we should increase the solver sell token balance by > the needed amount. Technically a solver could have private inventory > they would like to use for the solution instead of transfering it to > the trader. Because of this, it is not safe to override or use the `solver`'s sell token balance (it might be used during the settlement). This does mean that we need "simulation magic" for this feature to work in a way that does not intefere with the settlement.
nlordell
force-pushed
the
feat/token-state-override-2
branch
from
67fc567 to
7801a08
Compare
nlordell
force-pushed
the
feat/token-state-override-3
branch
from
83ae3bb to
f7fcfff
Compare
1 task
MartinquaXD
approved these changes
Dec 5, 2024
4 tasks
|
Superseded by cowprotocol#3147 |
MartinquaXD
pushed a commit
to cowprotocol/services
that referenced
this pull request
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 🤷. - [x] Adds command line configuration for token balance overrides - [x] Adds logic to the trades verifier to setup state overrides for take token balances for the configured tokens - [x] Adds logic in the settlement simulation to try and fund the trader if they are missing balances and balance overrides are enabled - [x] 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: > * nlordell#1 > * nlordell#2.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR builds on #1 and uses a separate account for pre-funding the trader with sell tokens in case they don't have a sufficient balance. This was done to interfere less with the settlement process:
Because of this, it is not safe to override or use the
solver's sell token balance (it might be used during the settlement). This does mean that we need "simulation magic" for this feature to work in a way that does not intefere with the settlement.Changes
A new
Spardose(piggy bank) contract is added that can be used for funding and ensuring the trader has a specific balance in case mocking is enabled for the simulation. This contract gets a balance override for the sell token instead of theSolveraccount to avoid interfering with the settlement.How to test
The existing E2E test on verified quotes with simulated balances continues to pass.