Fix 4626 classification

[fleupold]

Description

Wintermute is looking to enable quotes for their Wildcat issued wmtUSDx tokens (e.g. wmtUSDC). Currently those tokens fail quoting because we cannot find a native price. The reason for this is that the token looks like an ERC-4626 vault token (ie. it implements assets()) but doesn't follow the standard (ie it doesn't implement convertToAssets()).

In our current logic failure to convert gets bubbled up directly as an estimation error (without attempting the standard ERC20 estimator path)

Changes

• Treat tokens for which conversion fails like non 4626 assets (fallback to ERC20 estimators)
• Also cache failure to convert so we don't probe the same token over and over again

How to test

Added unit test

[claude]

Claude finished @fleupold's task in 2m 33s —— View job

---

PR Review: Fix 4626 classification

• Read changed file and diff against base
• Verified is_contract_revert() semantics for the failure mode
• Reviewed test coverage and edge cases

Verdict: LGTM ✅

Clean, well-scoped fix. The logic correctly distinguishes a contract-level rejection (partial/non-conforming EIP-4626 token) from a transient transport failure:

• fetch_conversion_data now returns Ok(None) when convertToAssets() fails with is_contract_revert() and Err otherwise. That predicate (ethrpc/src/alloy/errors.rs:60) also matches ZeroData / UnknownFunction / empty-data reverts, so a token like wmtUSDC that exposes asset() but lacks a working convertToAssets() selector is correctly caught and falls back to the ERC-20 estimator path.
• Transient failures still bubble up as Err, so they retry rather than getting pinned as non-vault — the negative cache only records deterministic classifications. Good.
• The mark_non_vault extraction removes the duplicated insert + metric update. Docs are updated to match.
• The new test drives asset()/decimals() success + reverting convertToAssets() and asserts both delegation to the inner estimator and that the classification is cached.

Minor (non-blocking, pre-existing)

The negative cache is process-lifetime and never evicted. A legitimate vault that is temporarily reverting on convertToAssets() (e.g. paused via require) would surface as a contract revert and be permanently pinned as non-vault until restart. This edge case already applied to the asset()-revert caching, so it's not introduced here — just worth being aware of. No change requested.
· fix/4626_classification