Merge/upstream main 2025 11 18 - #19
Merged
Merged
Conversation
# Description Plotting the entire (or at least vast majority) of time lost just running the auction (i.e. everything besides actually computing solutions) is extremely important for guiding our optimization efforts. We already have some metrics for that but since those are histograms we have a few issues: 1. the granularity of histograms depends on the buckets we define. The necessary granularity can vary a lot depending on the task so reusing the same metric for multiple sources of overhead either means we have to introduce a TON of buckets or multiple histograms (one for each source of overhead). 2. AFAIK histograms can't be merged into 1 nice plot that visualizes all the overhead at once. Instead you basically have to look at each histogram individually and mentally piece everything together. # Changes This PR addresses both issues by measuring the overhead using 2 counters. One for measuring the total time spent in each phase and one for counting how many measurements we did. Using gauges for this would have been a bit easier but gauges have the issue that they only plot the exact value stored at the time when prometheus scrapes the metrics. Since the runtime of the individual sources of overhead can vary quite a bit from run to run there is a chance that gauges misrepresent the metrics. With the 2 counter approach we can at least always compute averages for all sources of overhead which should hopefully give us better data. As we continue to reduce this overhead it might make sense to break down some of these phases a bit more but I think this is a good starting point. Note that a lot of plotted phases look insignificant in my screenshot but only because the data comes from the playground which basically does nothing. From my previous efforts to optimize performance I know that many of these phases take a surprising amount of time. ## How to test I used cowprotocol#3752 to build the new dashboard I want to build in the playground to verify that things work as I intend. As you can see that dashboard makes it a lot easier to get a sense of ALL the auction overhead at once and how much each phase contributes to the total overhead. <img width="1247" height="639" alt="Screenshot 2025-10-09 at 06 32 57" src="https://github.com/user-attachments/assets/74196838-74fc-4188-a5b9-fd8775eb5d1d" />
# Description Migrate Counter into alloy # Changes <!-- List of detailed changes (how the change is accomplished) --> - [ ] Remove old bindings - [ ] Add new bindings - [ ] Adapt tests ## How to test Tests <!-- ## Related Issues Fixes # --> --------- Co-authored-by: ilya <ilya@cow.fi>
Updates the DB reminder message, since currently 2 auctions are running in parallel on each deployment, and this should be taken into account when committing DB-breaking changes.
# Description As it [was suggested](cowprotocol#3762 (review)) in the previous PR: > Fine to merge as is but I just realized that this test is actually just verifying that both hooks get executed but not that they get executed but not WHEN they get executed. > In order to verify that we could adjust the Counter to set the count to the user's sell token balance at the time of executing the hook. Then we'd assert that the pre counter has the value of the user's sell token balance before the settlement starts and post has the value of afterwards. Since the value the count is set to depends on data that changes throughout the settlement we'd know when the hook was actually executed. # Changes - Update the test Counter helper so hooks can overwrite a counter with a live ERC20 balance snapshot. - Adjust the Counter ABI JSON to add the new function while leaving the original structure intact. To better understand the change, use this diff: cowprotocol@9cf8761 - Rework the partial-fills hook e2e test to assert pre/post hook execution timing by checking WETH balances recorded by the new helper. --------- Co-authored-by: José Duarte <duarte.gmj@gmail.com>
# Description
`tokio-console` can be nice to see what's going on in the tokio runtime.
This PR makes it so that all services pods will be spawned with tokio
console enabled.
It also fixes a few other playground issues
# Changes
For Tokio Console
- adjusts `Dockerfile` to add `--cfg tokio_unstable` to
`.cargo/config.toml` which builds the services with `tokio-console`
support
- adds necessary tokio console env variables to docker compose files
- `TOKIO_CONSOLE=true` to activate the feature in our processes
- `TOKIO_CONSOLE_RETENTION=600s` to limit the memory used for all the
metrics
- `TOKIO_CONSOLE_BIND=0.0.0.0:6669` to open port listening on all
devices (the default of `127.0.0.1` can not be reached from outside
docker)
- adds the necessary port forwards (while avoiding conflicts)
- moved the log about the tokio configuration message until after the
subscriber gets initialized - otherwise it doesn't actually get logged 😅
- added `tokio-console` instructions in the readme
Other stuff
- convert `as` to `AS` to avoid docker complaining about inconsistent
capitalization
- removed caching from `yarn` build step of frontend and explorer
because having multiple builds use the same yarn cache regularly caused
errors in my builds. Since we usually don't rebuild the frontends not
having the cache here doesn't slow anything down AFAICS
## How to test
1. Install [tokio-console](https://github.com/tokio-rs/console)
2. start playground
3. run `tokio-console` (this connects to the default port which I
assigned to the orderbook).
Other pods can be accessed with `tokio-console http://localhost:<PORT>`
The port mappings are as follows:
orderbook: 6669
autopilot: 6670
driver: 6671
baseline: 6672
<img width="962" height="498" alt="Screenshot 2025-10-10 at 10 06 11"
src="https://github.com/user-attachments/assets/31e93560-004b-42d4-88b7-3639a068ce5b"
/>
# Description Updates alloy to v1.0.38, which contains quite a lot updates: alloy-rs/alloy@v1.0.36...v1.0.38
…protocol#3766) # Description The `auction_participans` table has become obsolete, since the same data is now stored in the `proposed_solutions`. This PR migrates usage to the latter table. The migration script will be a part of cowprotocol#3753, since this is a breaking change, which requires gradual deployment to avoid panics when running 2 auctions in parallel.
# Description After the CoWSwapEthFlow SC was migrated to alloy, we've started receiving the following errors: ``` 2025-10-14T16:46:08.171Z WARN refunder: Error while refunding ethflow orders: local usage error: Missing signing credential for 0x0214aE5fD178986fA18ff792e0b995Dc6a78cD56 Caused by: Missing signing credential for 0x0214aE5fD178986fA18ff792e0b995Dc6a78cD56 ``` That basically means that our refunder doesn't work. The reason for that is that the refunder's private key wasn't set properly in the alloy provider. This PR fixes it. ## How to test Probably not easy.
Co-authored-by: Marcin Szymczak <mail@marszy.com> Co-authored-by: ilya <ilya@cow.fi>
# Description Required for tx gas configuration to be backwards compatible and guarantee smooth deployment. The infrastructure will be able to set tx gas limit per chain, but unless this field is expected, the driver would just panic on parsing the config. This introduces the required field as an option, and does not do anything with the value if set. cowprotocol#3780 will actually make it mandatory and work.
Required to fix Sepolia after fusaka hard fork. # Description Sepolia got broken because the fusaka hard fork introduced a protocol level cap on tx gas limit which currently is being set to the block gas limit (which is too high). # Changes Make tx gas limit configurable in the driver, preserving the old behaviour (of taking block gas limit) if it is not specified. - [ ] Add tx_gas_limit command line argument (env: TX_GAS_LIMIT) ## How to test 1. Configure limit to 2^24 - 1 and test if transactions can be made on Sepolia 2. ## Related Issues Fixes cowprotocol#3777
…owprotocol#3753) Actually drops the table from the DB. More details can be found in previous PRs: cowprotocol#3751, cowprotocol#3766. Must be released separately from the mentioned PRs.
# Description This PR drops the CIP-20 data, which has become obsolete. # Changes - [ ] univ2 test still uses this data to validate it is updated in some way, since we don't have other test to cover that. - [ ] Instead of using the CIP-20 structs, the queries migrated to separate function. ## How to test Updated existing tests.
# Description Migrates BalancerV3BatchRouter to alloy. Alloy, by default, wasn't able to parse the ABI JSON we currently have, so I had to update it a bit. To see the exact change, see [this](cowprotocol@d4db354#diff-6b43d7a940f1bbb3e887415e980543e0825ea48d1d5919797012f188f71b8551) commit after the formatting one. This would require changes in the gnosis/solvers repo, since the SC is used only there.
# Description This is a follow-up to cowprotocol#3779, which implements @jmg-duarte's proposal to set the refunder's wallet when creating the alloy transport, rather than doing it on each relevant function call. # Changes - Replace ethcontract `Account` with alloy's `TxSigner` - Register the signer with the Web3 transport during RefundService initialization - Simplify transaction submission by removing per-call signer setup
Co-authored-by: ilya <ilya@cow.fi>
# Description We’ve seen occasional race conditions between cancellation and settlement transactions caused by how the `web3` library handles nonces, which currently happens on Base. The `web3` lib [fetches](https://github.com/tomusdrw/rust-web3/blob/190c21de7ebf8a3ad58a541aa3175300d3a3f36a/src/api/accounts.rs#L93-L97) the [Latest](https://github.com/tomusdrw/rust-web3/blob/19cc946650eb1400526b7a98ee15fa4fc21813c8/src/api/eth.rs#L214) nonce when it is None, which is always the case with our current implementation(we don't set nonce anywhere in the driver crate, the same happens in the ethcontract-rs lib[[code](https://github.com/cowprotocol/ethcontract-rs/blob/da717d73557c7448467a34ff169cdf2ceaac1810/ethcontract/src/transaction/build.rs#L232-L233)]). This can result in using a stale nonce when two txs are sent close together. Switching to Pending makes the node include txs already accepted into the mempool when reporting the next nonce. This should be enough to prevent most of these race conditions, since the cancel will immediately bump the pending nonce and the next settle will use the correct one. The alloy lib already uses it by default. https://github.com/alloy-rs/alloy/blob/93d1c98cc841720b5fcb4c577d6881cb51275afe/crates/provider/src/fillers/nonce.rs#L35-L45 There’s still a chance for internal propagation lag: even via the same RPC, the node’s mempool view can momentarily lag its submission path / upstream sequencer, so pending might not reflect the just-submitted tx for a brief window. This should be much rarer than the latest-vs-pending race we're fixing. ## How to test Staging to ensure it works fine and then chain by chain on prod. ## Further implementation A proper fix would be a local nonce management in the driver (per-address cache + sync logic), since only the driver hass access to solvers private keys, but that’s more complex to get right. Even though alloy already provides [CachedNonceManager](https://docs.rs/alloy/latest/alloy/providers/fillers/struct.CachedNonceManager.html), it can easily go out of sync since it optimistically updates the local cache without ensuring whether the tx was mined and the nonce is updated, so it would require a much more sophisticated approach.
# Description As it was suggested in [another PR comment](cowprotocol#3781 (comment)), the `MutWallet::register_signer()` function doesn't require a mutable reference to self. This PR fixes this. # Changes Change `MutWallet::register_signer()` to take `&self` instead of `&mut self`. Since the wallet is internally wrapped in `Arc<RwLock<>>`, mutation through a shared reference is safe and intended.
# Description @marcovc reported that enabling appdata fetching in the driver leads to very slow driver restarts. Since we used `join_all` the driver will only continuing the auction pre-processing if ALL futures finished. Because drivers running in the same k8s cluster as the orderbook have significantly lower latency we never saw that issue. # Changes In order to not block the pre-processing for an unreasonable amount of time I adjusted the logic to only await new appdatas for 500ms. That should give it enough time to fetch completely new appdatas we've never seen before (basically every new auction introduces at most a couple of new orders + appdatas). If that were the only change we would likely need MANY auctions to completely fill the cache with only 500ms per auction. To address that issue I adjusted the appdata fetcher to spawn 1 tokio task per appdata that needs to be fetched. That way the caller can await however many requests they want and all the ones they didn't wait for will still make progress while their solver already computes a solution for the current auction. In the next auction all (or at least many) of the missing appdata values should already be available in the cache. ## How to test e2e tests should already cover that appdatas become available eventually.
# Description `ethrpc` only depends on 1 function from `contracts` which arguably shouldn't have been re-exported to begin with since it's only needed for `ethcontract` bindings. Removing that function from `ethrpc` means it no longer depends on `contracts` which allows `cargo` to compile it in parallel with `contracts`. Also where this `dummy` functionality lives become irrelevant when we finally switch to `alloy` completely since `alloy` does not need these dummy instances just to encode calldata. 🥳 # Changes - remove `contracts` dependency from `ethrpc` ## How to test compiler
# Description @jmg-duarte noticed that there are crates significantly faster than `hex` for de/encoding bytes (10-50x). Since a lot of our time is spent (de)/serializing bytes and addresses this should give us a nice free performance boost. # Changes - replaced `hex` with `const_hex` everywhere - replace handrolled `0x` prefixing with `const_hex::encode_prefixed` ## How to test e2e tests should be sufficient to cover these changes
# Description We're sometimes getting "insufficient funds" reverts with ridiculous estimates in access lists. Adding some instrumentation that captures the return value of the gas estimator to be able to debug this better.
# Description Migrates the rest of the cow_amm to alloy # Changes <!-- List of detailed changes (how the change is accomplished) --> - [ ] Removes the ethcontract dep - [ ] Replaces use on re-export ## How to test Tests <!-- ## Related Issues Fixes # -->
# Description Migrate trade_finding::Quote to alloy # Changes <!-- List of detailed changes (how the change is accomplished) --> - [ ] Migrates the Quote to alloy - [ ] Refactors where needed ## How to test Tests <!-- ## Related Issues Fixes # -->
# Description Migrate BadTokenDetecting to alloy # Changes <!-- List of detailed changes (how the change is accomplished) --> - [ ] Migrate BadTokenDetecting to alloy - [ ] Migrates the arguments that power the detectors too - [ ] Refactors where applicable ## How to test Tests <!-- ## Related Issues Fixes # -->
# Description Migrate price_estimation::Query to alloy # Changes <!-- List of detailed changes (how the change is accomplished) --> - [ ] Migrate price_estimation::Query to alloy - [ ] Refactors where applicable ## How to test Tests <!-- ## Related Issues Fixes # -->
# Description Migrate token_list to alloy # Changes <!-- List of detailed changes (how the change is accomplished) --> - [ ] Migrate token_list to alloy - [ ] Includes some "upstream" friends in the arguments - [ ] Refactors where applicable ## How to test Tests <!-- ## Related Issues Fixes # -->
# Description Migrate quote DTO to alloy # Changes <!-- List of detailed changes (how the change is accomplished) --> - [ ] Migrates the quote DTO inner types to alloy - [ ] Applies refactors where needed ## How to test Existing tests <!-- ## Related Issues Fixes # -->
# Description Migrate driver::util::math into alloy The replacement of U256s implies that we no longer have the same operations available and need to implement some by hand. Notes on operations: * full_mul -> widening_mul (kudos @squadgazzz) # Changes - [ ] Migrate driver::util::math into alloy - [ ] Refactors where applicable - [ ] Adds unit tests ## How to test N/A
…rotocol#3882) # Description Migrate the native price estimation endpoint and trait to alloy # Changes <!-- List of detailed changes (how the change is accomplished) --> - [ ] Migrates v1/token/{addr}/native_price endpoint to alloy - [ ] Migrates the NativePriceEstimating trait to alloy - [ ] Refactors the tests and rest of codebase ## How to test Existing tests <!-- ## Related Issues Fixes # -->
# Description Migrate get_orders_by_tx endpoint to alloy # Changes <!-- List of detailed changes (how the change is accomplished) --> - [ ] Migrates the endpoint types to alloy - [ ] Refactors were applicable ## How to test Existing tests <!-- ## Related Issues Fixes # -->
# Description Migrate solver competition endpoint to alloy # Changes <!-- List of detailed changes (how the change is accomplished) --> - [ ] Replaces the ethcontract::H256 with alloy::primitives::B256 - [ ] Refactors where applicable ## How to test Existing tests <!-- ## Related Issues Fixes # -->
# Description Migrate get_token_metadata endpoint to alloy # Changes <!-- List of detailed changes (how the change is accomplished) --> - [ ] Fixes some editor highlights for tests in the orderbook - [ ] Migrates the get_token_metadata endpoint to alloy - [ ] Refactors where needed ## How to test Existing tests <!-- ## Related Issues Fixes # -->
# Description Migrate get_total_surplus_endpoint to alloy # Changes <!-- List of detailed changes (how the change is accomplished) --> - [ ] Migrate get_total_surplus_endpoint to alloy - [ ] Refactor where needed ## How to test Existing tests <!-- ## Related Issues Fixes # -->
# Description Migrate get_trades endpoint to alloy # Changes <!-- List of detailed changes (how the change is accomplished) --> - [ ] Migrate get_trades endpoint to alloy - [ ] Refactor related structures ## How to test Existing tests <!-- ## Related Issues Fixes # -->
# Description Migrate get_user_orders endpoint to alloy # Changes <!-- List of detailed changes (how the change is accomplished) --> - [ ] Migrate get_user_orders endpoint to alloy - [ ] Refactor the related structures ## How to test Existing tests <!-- ## Related Issues Fixes # -->
# Description Migrate orderbook::database::auction_prices to alloy # Changes <!-- List of detailed changes (how the change is accomplished) --> - [ ] Migrates the auction_prices module to alloy - [ ] Refactors where needed ## How to test Existing tests <!-- ## Related Issues Fixes # -->
# Description We want to disable order filtering for flash loans, but keep filtering for presign order that have not been presigned. For this I introduced a new flag that forces this filter even if filtering in general is disabled.
## Problem The refunder was experiencing issues with hardcoded gas price parameters that were too low for current network conditions: ### Errors observed: 1. `Refunding txs are likely not mined in time, as the current gas price 826120518137.4663 is higher than MAX_GAS_PRICE specified 800000000000` 2. `transaction gas price below minimum: gas tip cap 5773015156, minimum needed 25000000000` ### Root cause: - `MAX_GAS_PRICE` was hardcoded to 800 Gwei (current gas prices ~826 Gwei) - `START_PRIORITY_FEE_TIP` was hardcoded to 2 Gwei (node requires minimum 25 Gwei) ## Solution This PR makes both gas price parameters configurable via CLI arguments or environment variables: - `--max-gas-price` / `MAX_GAS_PRICE` (default: 2000 Gwei / 2,000,000,000,000 wei) - `--start-priority-fee-tip` / `START_PRIORITY_FEE_TIP` (default: 30 Gwei / 30,000,000,000 wei) The new defaults are set higher to accommodate current network conditions while still being configurable for future adjustments. ## Changes - Added two new CLI arguments with environment variable support - Refactored `Submitter` and `RefundService` to accept configurable parameters - Updated tests to use test-specific constants - Updated argument display formatting to show new parameters ## Testing - [x] Code compiles successfully (`cargo check -p refunder`) - [x] No linter errors - [x] Unit tests updated and passing --------- Co-authored-by: ilya <ilya@cow.fi>
# Description We are trying to accommodate flashloan testing by disabling balance and signature filtering in the autopilot. The problem is that it is legitimate to not have the required sell token balance as it comes from the flashloan executed in pre-interactions. Similarly it is legitimate that on 1271 orders the signature checks only succeeds after pre-interactions with the flashloan have executed. By disabling these checks in the autopilot we are effectively moving them onto the driver and potentially introducing performance problems in it. # Changes - [x] Removed `disable_order_filtering` flag that would switch off both balance and signature filtering - [x] Introduced `disable_order_balance_filter` flag to control filtering on insufficient balance - [x] Introduced `disable_1271_order_sig_filter` to disable filtering on signatures of 1271 orders
…#3902) # Description A continuation of cowprotocol#3901. We want to introduce more control and be able to switch off only 1271 balance checks in the autopilot. In practice it is going to mean that most order will be filtered out by the autopilot.
# Description Typo in a comment.
# Description
We ran into an issue where CoinGecko was enabled as native price
estimator, but there the necessary env var with API key was not set. To
prevent this in the future we want to have the service not start
instead. Otherwise one gets weird behaviour when the service runs where
sometimes we get native prices when some solvers decide to quote them,
but not always and that leads to order that would otherwise be solved
never get created in the first place as one can't get a quote ("no
liquidity" error).
# Description Adds protocol volume fee support to the orderbook quote API in order to reflect the auction fees after submitting an order using the same quote. The quote response now includes `protocolFeeBps` ~~and `protocolFeeSellAmount`~~ field~~s~~ when volume fees are configured, allowing users to see the fee before placing orders. Volume fees are applied to the surplus token (buy token for sell orders, sell token for buy orders) following the same logic as the driver. The orderbook adjusts quote amounts so that orders can be signed with amounts that will be fillable after the driver applies fees during auction competition. The PriceImprovement and Surplus fees are based on the quotes, so it doesn't make any sense to support them in the quote API. # Changes - Added `--volume-fee` CLI argument to orderbook (e.g., `--volume-fee=0.0002` for 0.02% or 2 basis points) - Accepts decimal values in range `[0, 1)` representing the fee factor - Added optional `protocolFeeBps` (string) ~~and `protocolFeeSellAmount` (U256)~~ field~~s~~ to `OrderQuoteResponse` - The field~~s~~ are only present when `--volume-fee` is configured in the orderbook # Implementation details - Volume fee calculation follows driver logic: https://github.com/cowprotocol/services/blob/31ea719f35072bc6155fdeb990bbc27c9b8833b3/crates/driver/src/domain/competition/solution/fee.rs#L185-L202 - **Sell orders**: Fee calculated on `buy_amount`, reduces the buy amount returned in quote - **Buy orders**: Fee calculated on `sell_amount` + network fee, increases the sell amount returned in quote - ~~Fee amounts are always converted to sell token for the `protocolFeeSellAmount` field~~ - ~~Uses `quote.sell_amount/buy_amount` (final computed amounts after network fees) rather than `quote.data.quoted_sell_amount/quoted_buy_amount` (original exchange rate amounts)~~ ## How to test New unit and e2e tests. ## Further configuration This feature needs to be enabled only on a specific block. This logic will be implemented in a follow-up PR. --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
# Description cowprotocol#3900 introduced a way to show the volume fees to users in advance via quotes. Autopilot should start applying the same policies simultaneously to avoid situations where the user receives an incorrect quote. This PR introduces timestamp-based configs in both orderbook and autopilot crates that control when the volume fee should start applying. For the orderbook, it is pretty straightforward, which adds an optional timestamp param to the existing volume fee factor config, and each time the service tries to apply volume fees, it checks for the current time. If the timestamp config is None, it means volume fees are applied unconditionally, which should be useful once switched to a long-lasting config. The autopilot config is a bit more sophisticated. It introduces a separate "upcoming" fee policies config with the effective "from" timestamp. ~~So, if another order's creation timestamp is after the configured upcoming fee policy timestamp, the service starts using this fee policy.~~ Based on [this discussion](cowprotocol#3907 (comment)), the volume fee gets applied only based on the current time. This is useful because the volume fee factor affects price improvement and surplus fee policy caps, so each time the volume fee factor is updated, other fee policy configs need to be adjusted accordingly, so we need to switch to the new fee policies set altogether. The config is also optional and can be easily switched to permanent. The major disadvantage of this approach is that orderbook and autopilot use configs from different sources. A more correct approach would be to use a shared config via a DB or similar, but this would require many more changes, and we should probably avoid any mistakes by making deeper reviews. ## How to test New e2e tests. --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
# Description My apologies for the long diff. Migrating the endpoints requires migrating some of these structures which leads to changes like this. # Changes <!-- List of detailed changes (how the change is accomplished) --> - [ ] Migrates the model::OrderData to alloy - [ ] Refactors the code downstream from the struct ## How to test Existing tests <!-- ## Related Issues Fixes # -->
|
I have read the CLA Document and I hereby sign the CLA 0 out of 11 committers have signed the CLA. |
This file contains hidden or 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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
13 participants
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
Changes
How to test