Repository: ApeWorX/ape

Issue:
Call trace enrichment method pollutes `contract_type` JSON files for unrelated contracts when `tx.show_trace()` includes an ERC-20 token
### Have you ensured that all of these are up to date?

- [x] Ape
- [ ] Ape plugins

### What version of Ape are you on?

0.8.29

### What command(s) is the bug in?

ape test

### Operating System

Linux

### Ape Configuration

n/a

### Description

I am developing an arbitrage contract that swaps between Uniswap V2 and V4. While debugging various failures, I wrote an Ape test that called the main execution method with the argument `raise_on_revert=False` followed by a check for failure, and a `tx.trace()` if the transaction reverted. 

After some time the tests began to stall, eventually failing to complete within an hour, and the last run killed by the out-of-memory watchdog on my machine.

The `contract_types` directory was quite large, roughly 3 GB. I cleared that directory and ran the test again, finding that the trace was fast. Repeated runs of the test revealed that certain contract_type files were growing in size:

```
btd@dev:~/.ape/ethereum/mainnet/contract_types$ du -sh *
92K    0x000000000004444c5dc75cB358380D2e3dE08A90.json
56K    0x2260FAC5E5542a773Aa44fBCfeDf7C193bc2C599.json
68K    0xBb2b8038a1640196FbE3e38816F3e67Cba72D940.json

btd@dev:~/.ape/ethereum/mainnet/contract_types$ du -sh *
1.0M    0x000000000004444c5dc75cB358380D2e3dE08A90.json
600K    0x2260FAC5E5542a773Aa44fBCfeDf7C193bc2C599.json
664K    0xBb2b8038a1640196FbE3e38816F3e67Cba72D940.json

btd@dev:~/.ape/ethereum/mainnet/contract_types$ du -sh *
12M    0x000000000004444c5dc75cB358380D2e3dE08A90.json
6.8M    0x2260FAC5E5542a773Aa44fBCfeDf7C193bc2C599.json
7.4M    0xBb2b8038a1640196FbE3e38816F3e67Cba72D940.json
```

I suspected some corruption of the contract type cache, and ultimately discovered that the `_enrich_contract_id` method in ecosystem.py causes the JSON to be overwritten with invalid data:

```
if kwargs.get("use_symbol_for_tokens") and "symbol" in contract_type.view_methods:
    # Use token symbol as name
    contract = self.chain_manager.contracts.instance_at(
        address, contract_type=contract_type
    )
```

The `contract_type` object gets corrupted because the `_get_contract_type_for_enrichment` function prioritizes the contract type in `kwargs` instead of the cached contract_type held by the `chain_manager.contracts` container.

For long call traces involving token transfers early in the transaction, the token `contract_type` is placed in `kwargs` and corrupts all lookups after. 

In my case, the WETH9 contract was placed into kwargs in the first loop, and repeatedly found on subsequent enrichment runs by `_get_contract_type_for_enrichment`. When this method finds the 'contract_type' key set in `kwargs`, it skips the fetch from the `chain_manager.contracts` container. There is no address or name verification performed by this method, so the mismatch is not detected.

After this occured, WETH9 persisted in `kwargs`, and was returned as the `contract_type` for the enriched traces for V2 pool, V4 PoolManager, and my arbitrage contract. The `"symbol" in contract_type.view_methods` check triggered because 'symbol' is a standard ERC-20 method. This led to the WETH9 contract type being saved back to the cache in the call to `self.chain_manager.contracts.instance_at()`, since the `contract_type=` argument is provided.

I have naively resolved the issue by popping both 'contract_address' and 'contract_type' from `kwargs` at the top of `_enrich_contract_id` so subsequent lookups aren't corrupted.

I'm ignorant about other effects this might have, so I'm filing the issue now for discussion. 

Draft PR to follow.

### Reproduction code

_No response_

Discussion:
@linear: <p><a href="https://linear.app/apeworx/issue/APE-1913/call-trace-enrichment-method-pollutes-contract-type-json-files-for">APE-1913 Call trace enrichment method pollutes `contract_type` JSON files for unrelated contracts when `tx.show_trace()` includes an ERC-20 token</a></p>@fubuloubu: Thank you so much for the detailed breakdown! Will definitely accept a PR, let us know I'd you have issues with it or need help otherwise. Sounds like a serious issue@BowTiedDevil: I am working on the PR now and will use this as a scratch space to illustrate the problem.

I've made the following debug print addition to ecosystem.py:
```
if kwargs.get("use_symbol_for_tokens") and "symbol" in contract_type.view_methods:
    # Use token symbol as name
    print(f"Storing {repr(contract_type)} at address {address}")
    contract = self.chain_manager.contracts.instance_at(
        address, contract_type=contract_type
    )
```

Emptying the contract_types folder and running my arb test shows the following output:

```
btd@dev:~/code/uniswap_v4_v2_executor$ uv run ape test -s tests/test_executor.py::test_execute_case_one                  
================================================== test session starts ==================================================
platform linux -- Python 3.13.1, pytest-8.3.5, pluggy-1.5.0 -- /home/btd/code/uniswap_v4_v2_executor/.venv/bin/python3   
cachedir: .pytest_cache                                                                                                  
rootdir: /home/btd/code/uniswap_v4_v2_executor                                                                           
plugins: eth-ape-0.1.dev1578+g6a054a5                                                                                    
collected 1 item                                                                                                         
                                                                                                                         
tests/test_executor.py::test_execute_case_one BTC_FORWARD_AMOUNT=1000000                                                 
v2_btc_reserves=6458188307                                                                                               
v2_weth_reserves=2623174595354055869576                                                                                  
v2_weth_in=407463384008495324                                                                                            
ERROR:    revert: Balance reduction                                                                                      
Getting TX trace...                                                                                                      
Storing <ContractType WETH9> at address 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2                                       
Storing <ContractType WETH9> at address 0xFf8FA9381caf61cB3368a6ec0b3F5C788028D0Cd                                       
Storing <ContractType v4_v2_executor> at address 0xFf8FA9381caf61cB3368a6ec0b3F5C788028D0Cd                              
Storing <ContractType WETH9> at address 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2                                       
Storing <ContractType WETH9> at address 0xFf8FA9381caf61cB3368a6ec0b3F5C788028D0Cd                                       
Storing <ContractType WETH9> at address 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2                                       
Storing <ContractType WETH9> at address 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2                                       
Storing <ContractType WETH9> at address 0xBb2b8038a1640196FbE3e38816F3e67Cba72D940                                       
Storing <ContractType WBTC> at address 0x2260FAC5E5542a773Aa44fBCfeDf7C193bc2C599                                        
Storing <ContractType WBTC> at address 0x000000000004444c5dc75cB358380D2e3dE08A90                                        
Storing <ContractType PoolManager> at address 0x000000000004444c5dc75cB358380D2e3dE08A90                                 
Storing <ContractType PoolManager> at address 0x2260FAC5E5542a773Aa44fBCfeDf7C193bc2C599                                 
Storing <ContractType WBTC> at address 0x2260FAC5E5542a773Aa44fBCfeDf7C193bc2C599                                        
Storing <ContractType WBTC> at address 0x000000000004444c5dc75cB358380D2e3dE08A90                                        
Storing <ContractType WBTC> at address 0x2260FAC5E5542a773Aa44fBCfeDf7C193bc2C599                                        
Storing <ContractType WBTC> at address 0xBb2b8038a1640196FbE3e38816F3e67Cba72D940                                        
Storing <ContractType WETH9> at address 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2                                       
Storing <ContractType WETH9> at address 0xBb2b8038a1640196FbE3e38816F3e67Cba72D940                                       
Storing <ContractType UniswapV2Pair> at address 0xBb2b8038a1640196FbE3e38816F3e67Cba72D940                               
Storing <ContractType UniswapV2Pair> at address 0x000000000004444c5dc75cB358380D2e3dE08A90                               
Storing <ContractType WBTC> at address 0x2260FAC5E5542a773Aa44fBCfeDf7C193bc2C599                                        
Storing <ContractType WBTC> at address 0x000000000004444c5dc75cB358380D2e3dE08A90                                        
Storing <ContractType PoolManager> at address 0x000000000004444c5dc75cB358380D2e3dE08A90                                 
Storing <ContractType v4_v2_executor> at address 0xFf8FA9381caf61cB3368a6ec0b3F5C788028D0Cd                              
Storing <ContractType PoolManager> at address 0x000000000004444c5dc75cB358380D2e3dE08A90                                 
Storing <ContractType WETH9> at address 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2
Storing <ContractType WETH9> at address 0xFf8FA9381caf61cB3368a6ec0b3F5C788028D0Cd
Storing <ContractType v4_v2_executor> at address 0xFf8FA9381caf61cB3368a6ec0b3F5C788028D0Cd
Storing <ContractType v4_v2_executor> at address 0xBb2b8038a1640196FbE3e38816F3e67Cba72D940
Storing <ContractType v4_v2_executor> at address 0x2260FAC5E5542a773Aa44fBCfeDf7C193bc2C599
Call trace for '0xbea0862eca1dd90de986c094eee8a38553864b2b20f4a3b6528944d5712bc491'
reverted with message: "Balance reduction"
tx.origin=0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266
v4_v2_executor.execute(
  v2_payload=['UNI-V2', 0, 407463384008495324, 1000000], 
  v4_payload=[
    ZERO_ADDRESS,
    WBTC,
    3000,
    60,
    ZERO_ADDRESS,
    -1000000,
    0
  ]
) -> 
0x08c379a0000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000
0000000000000000000000000000000000001142616c616e636520726564756374696f6e0000000000000000000000000000
00 [135487 gas]
├── WETH.balanceOf(WETH9) -> 10000000000000000 [2534 gas]
├── PoolManager.unlock(data="") -> "" [124250 gas]
│   └── v4_v2_executor.unlockCallback(data="") -> "" [122303 gas]
│       ├── PoolManager.swap(
│       │     key=[
│       │       ZERO_ADDRESS,
│       │       PoolManager,
│       │       3000,
│       │       60,
│       │       ZERO_ADDRESS
│       │     ], 
│       │     params=[
│       │       0,
│       │       -1000000,
│       │       1461446703485210103287273052203988822378723970341
│       │     ], 
│       │     hookData=""
│       │   ) -> 137756486667348973197342150267687467159124925202859867584 [28582 gas]
│       ├── PoolManager.take(
│       │     currency=ZERO_ADDRESS, 
│       │     to=PoolManager, 
│       │     amount=404829929666486213
│       │   ) [8235 gas]
│       │   └── v4_v2_executor.0x() 404829929666486213 [107 gas]
│       ├── WETH.balanceOf(WETH9) -> 10000000000000000 [534 gas]
│       ├── WETH.deposit() 397463384008495325 [4874 gas]
│       ├── WETH.transfer(dst=UNI-V2, wad=407463384008495324) -> 1 [8062 gas]
│       ├── PoolManager.sync(currency=WBTC) [6668 gas]
│       │   └── WBTC.balanceOf(_owner=WBTC) -> 15288484892 [2795 gas]
│       ├── UNI-V2.swap(amount0Out=1000000, amount1Out=0, to=UniswapV2Pair, data="") [48355 gas]
│       │   ├── WBTC.transfer(_to=WBTC, _value=1000000) -> 1 [13703 gas]
│       │   ├── WBTC.balanceOf(_owner=UNI-V2) -> 6457188307 [795 gas]
│       │   └── WETH.balanceOf(UNI-V2) -> 2623582058738064364900 [534 gas]
│       └── PoolManager.settle() -> 1000000 [2769 gas]
│           └── WBTC.balanceOf(_owner=WBTC) -> 15289484892 [795 gas]
└── WETH.balanceOf(WETH9) -> 1 [534 gas]
```

The first thing to notice is the frequency of `instance_at` prints where the wrong `contract_type` is set. 

The second is to observe the corruption of the enriched contract ID in the call trace. Examples:
- The argument to the first two `balanceOf` calls to WETH should be `v4_v2_executor`, since this contract checks its WETH balance before doing swaps and as needed to unwrap for native Ether transfers.
- The `to` argument to `take()` should be the arb contract `v4_v2_executor`, not `PoolManager`.
- The `_owner` argument to the first and third WBTC `balanceOf` call should be `PoolManager` (the second is correct)
- The `to` argument to the V2 pool's `swap` function is incorrect - it should be going to `PoolManager` instead.
- The WBTC transfer triggered by that same `swap` is marked as `to=WBTC`

There are probably more examples, but these illustrate the issue.@BowTiedDevil: Now I've made a simple fix to `_enrich_contract_id`:

```
def _enrich_contract_id(self, address: AddressType, **kwargs) -> str:
    kwargs.pop("contract_type", None)
    ...
```

Deleting contract_types and running again, here is the output:

```
btd@dev:~/code/uniswap_v4_v2_executor$ uv run ape test -s tests/test_executor.py::test_e
xecute_case_one                                                                         
================================= test session starts ==================================
platform linux -- Python 3.13.1, pytest-8.3.5, pluggy-1.5.0 -- /home/btd/code/uniswap_v4
_v2_executor/.venv/bin/python3                                                          
cachedir: .pytest_cache                                                                 
rootdir: /home/btd/code/uniswap_v4_v2_executor                                          
plugins: eth-ape-0.1.dev1578+g6a054a5                                                   
collected 1 item                                                                        
                                                                                        
tests/test_executor.py::test_execute_case_one BTC_FORWARD_AMOUNT=1000000                
v2_btc_reserves=6454101453                                                              
v2_weth_reserves=2624840789035884517999                                                 
v2_weth_in=407980414569509893                                                           
ERROR:    revert: Balance reduction                                                     
Getting TX trace...                                                                     
Storing <ContractType WETH9> at address 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2      
Storing <ContractType WBTC> at address 0x2260FAC5E5542a773Aa44fBCfeDf7C193bc2C599       
Storing <ContractType WETH9> at address 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2      
Storing <ContractType WETH9> at address 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2      
Storing <ContractType WETH9> at address 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2      
Storing <ContractType UniswapV2Pair> at address 0xBb2b8038a1640196FbE3e38816F3e67Cba72D9
40                                                                                      
Storing <ContractType WBTC> at address 0x2260FAC5E5542a773Aa44fBCfeDf7C193bc2C599       
Storing <ContractType WBTC> at address 0x2260FAC5E5542a773Aa44fBCfeDf7C193bc2C599       
Storing <ContractType WBTC> at address 0x2260FAC5E5542a773Aa44fBCfeDf7C193bc2C599       
Storing <ContractType WBTC> at address 0x2260FAC5E5542a773Aa44fBCfeDf7C193bc2C599       
Storing <ContractType UniswapV2Pair> at address 0xBb2b8038a1640196FbE3e38816F3e67Cba72D9
40                                                                                      
Storing <ContractType WETH9> at address 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2      
Storing <ContractType UniswapV2Pair> at address 0xBb2b8038a1640196FbE3e38816F3e67Cba72D9
40                                                                                      
Storing <ContractType UniswapV2Pair> at address 0xBb2b8038a1640196FbE3e38816F3e67Cba72D9
40                                                                                      
Storing <ContractType WBTC> at address 0x2260FAC5E5542a773Aa44fBCfeDf7C193bc2C599       
Storing <ContractType WETH9> at address 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2      
Storing <ContractType UniswapV2Pair> at address 0xBb2b8038a1640196FbE3e38816F3e67Cba72D9
40                                                                                      
Storing <ContractType WBTC> at address 0x2260FAC5E5542a773Aa44fBCfeDf7C193bc2C599       
Call trace for '0x2b7cf949ce435259001d6cbb853acf316afbb3bafe0148bd0adce12419b404ac'  
reverted with message: "Balance reduction"                                              
tx.origin=0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266
v4_v2_executor.execute(
  v2_payload=['UNI-V2', 0, 407980414569509893, 1000000], 
  v4_payload=[
    ZERO_ADDRESS,
    WBTC,
    3000,
    60,
    ZERO_ADDRESS,
    -1000000,
    0
  ]
) -> 
0x08c379a0000000000000000000000000000000000000000000000000000000000000002000000000000000
000000000000
0000000000000000000000000000000000001142616c616e636520726564756374696f6e0000000000000000
000000000000
00 [135483 gas]
├── WETH.balanceOf(v4_v2_executor) -> 10000000000000000 [2534 gas]
├── PoolManager.unlock(data="") -> "" [124246 gas]
│   └── v4_v2_executor.unlockCallback(data="") -> "" [122299 gas]
│       ├── PoolManager.swap(
│       │     key=[
│       │       ZERO_ADDRESS,
│       │       WBTC,
│       │       3000,
│       │       60,
│       │       ZERO_ADDRESS
│       │     ], 
│       │     params=[
│       │       0,
│       │       -1000000,
│       │       1461446703485210103287273052203988822378723970341
│       │     ], 
│       │     hookData=""
│       │   ) -> 137873410639736860963985484969586921364505992675701341632 [28578 gas]
│       ├── PoolManager.take(
│       │     currency=ZERO_ADDRESS, 
│       │     to=v4_v2_executor, 
│       │     amount=405173538339030371
│       │   ) [8235 gas]
│       │   └── v4_v2_executor.0x() 405173538339030371 [107 gas]
│       ├── WETH.balanceOf(v4_v2_executor) -> 10000000000000000 [534 gas]
│       ├── WETH.deposit() 397980414569509894 [4874 gas]
│       ├── WETH.transfer(dst=UNI-V2, wad=407980414569509893) -> 1 [8062 gas]
│       ├── PoolManager.sync(currency=WBTC) [6668 gas]
│       │   └── WBTC.balanceOf(_owner=PoolManager) -> 15276445680 [2795 gas]
│       ├── UNI-V2.swap(amount0Out=1000000, amount1Out=0, to=PoolManager, data="") [48355 gas]
│       │   ├── WBTC.transfer(_to=PoolManager, _value=1000000) -> 1 [13703 gas]
│       │   ├── WBTC.balanceOf(_owner=UNI-V2) -> 6453101453 [795 gas]
│       │   └── WETH.balanceOf(UNI-V2) -> 2625248769450454027892 [534 gas]
│       └── PoolManager.settle() -> 1000000 [2769 gas]
│           └── WBTC.balanceOf(_owner=PoolManager) -> 15277445680 [795 gas]
└── WETH.balanceOf(v4_v2_executor) -> 1 [534 gas]
```

Observe that now the contract types are being recorded at the correct address, and the bad IDs in the call trace above are now fixed.@BowTiedDevil: Now the remaining code block to clean up is this one, which always flushes the contract_type to disk for ERC-20 tokens (and similar) because 'symbol' is a method per the official spec:

```
if kwargs.get("use_symbol_for_tokens") and "symbol" in contract_type.view_methods:
    print(f"Storing {repr(contract_type)} at address {address}")
    contract = self.chain_manager.contracts.instance_at(
        address, contract_type=contract_type
    )
```

`contract_type` is in scope when this method executes, so the flushing to disk can be guarded with a check if it's already in the cache:

```
if kwargs.get("use_symbol_for_tokens") and "symbol" in contract_type.view_methods:
    if (contract := self.chain_manager.contracts.instance_at(address)) is None:
        print(f"Storing {repr(contract_type)} at address {address}")
        contract = self.chain_manager.contracts.instance_at(
            address, contract_type=contract_type
        )
``` 

I'll add this to the PR.


Solution patch:
diff --git a/src/ape_ethereum/ecosystem.py b/src/ape_ethereum/ecosystem.py
index 331c823cd..73a034ac7 100644
--- a/src/ape_ethereum/ecosystem.py
+++ b/src/ape_ethereum/ecosystem.py
@@ -1190,6 +1190,12 @@ class Ethereum(EcosystemAPI):
         return call
 
     def _enrich_contract_id(self, address: AddressType, **kwargs) -> str:
+        # Defensively pop "contract_type" key from kwargs. `_get_contract_type_for_enrichment` will
+        # preferentially return a `contract_type` from kwargs without checking the contract cache.
+        # The contract_type may not match the contract address being enriched if this method was
+        # previously called for a different contract.
+        kwargs.pop("contract_type", None)
+
         if address and address == kwargs.get("sender"):
             return "tx.origin"
 
@@ -1203,9 +1209,7 @@ class Ethereum(EcosystemAPI):
         kwargs["contract_type"] = contract_type
         if kwargs.get("use_symbol_for_tokens") and "symbol" in contract_type.view_methods:
             # Use token symbol as name
-            contract = self.chain_manager.contracts.instance_at(
-                address, contract_type=contract_type
-            )
+            contract = self.chain_manager.contracts.instance_at(address)
 
             try:
                 symbol = contract.symbol(skip_trace=True)


Test patch:
diff --git a/tests/functional/test_ecosystem.py b/tests/functional/test_ecosystem.py
index 3111cf2cb..6b36b7560 100644
--- a/tests/functional/test_ecosystem.py
+++ b/tests/functional/test_ecosystem.py
@@ -1134,7 +1134,7 @@ def test_enrich_trace(ethereum, vyper_contract_instance, owner):
 
 def test_enrich_trace_handles_call_type_enum(ethereum, vyper_contract_instance, owner):
     """
-    Testing a custom trace who's call tree uses an Enum type instead of a str.
+    Testing a custom trace whose call tree uses an Enum type instead of a str.
     """
 
     class PluginTxTrace(TransactionTrace):
@@ -1206,6 +1206,24 @@ def test_enrich_trace_handles_events(ethereum, vyper_contract_instance, owner):
     assert events == expected
 
 
+def test_enrich_trace_avoids_using_wrong_contract_type(
+    ethereum, vyper_contract_instance, solidity_contract_instance, owner, chain
+):
+    tx = vyper_contract_instance.setNumber(96247783, sender=owner)
+    trace = TransactionTrace(transaction_hash=tx.txn_hash)
+
+    # Purposely passing in the wrong contract type here.
+    actual = ethereum.enrich_trace(trace, contract_type=solidity_contract_instance.contract_type)
+
+    assert isinstance(actual, TransactionTrace)
+    assert actual._enriched_calltree is not None
+    assert re.match(r"VyperContract\.setNumber\(num=\d*\) \[\d* gas]", repr(actual))
+
+    # Ensure the contract type was not corrupted.
+    ct = chain.contracts[vyper_contract_instance.address]
+    assert ct == vyper_contract_instance.contract_type
+
+
 def test_get_deployment_address(ethereum, owner, vyper_contract_container):
     actual = ethereum.get_deployment_address(owner.address, owner.nonce)
     expected = owner.deploy(vyper_contract_container, 490)