feat: fermiswap integration#1034
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 33459105 | Triggered | Generic High Entropy Secret | cfdef47 | protocols/substreams/ethereum-fermiswap/substreams.yaml | View secret |
| 33671167 | Triggered | Generic High Entropy Secret | ff14c49 | protocols/substreams/ethereum-fermiswap/substreams.yaml | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
| format!("0x{}", hex::encode(out)) | ||
| } | ||
|
|
||
| pub fn lane_index(base_asset: &[u8], quote_asset: &[u8]) -> String { |
There was a problem hiding this comment.
Can you add docs that explains what this does with reference to the code please?
| if target != config.engine_address.as_slice() { | ||
| return None; | ||
| } | ||
| if update_timestamp.to_u64() != block_timestamp { | ||
| return None; | ||
| } |
There was a problem hiding this comment.
What's the logic behind these? Do we expect them or not and why is it safe to ignore them?
There was a problem hiding this comment.
I think I need to remove this check. In theory target should be engine_address and update_timestamp should match the current block, but the registry already allows some deviation here: (MAX_UPDATE_AGE and MAX_UPDATE_LEAD_TIME are both 0 now)
uint256 ts = updateTimestamp;
// Comparing a user-supplied timestamp against `block.timestamp` within a configurable
// window is the intended freshness check, not a vulnerability to miner manipulation.
// slither-disable-next-line timestamp
if (ts + MAX_UPDATE_AGE < block.timestamp) revert InvalidUpdateTimestamp();
// slither-disable-next-line timestamp
if (ts > block.timestamp + MAX_UPDATE_LEAD_TIME) revert InvalidUpdateTimestamp();it’s better to stay consistent with the onchain state.
| store.set_if_not_exists(0, lane_index(base_asset, quote_asset), &component.id); | ||
| store.set_if_not_exists(0, lane_index(quote_asset, base_asset), &component.id); |
There was a problem hiding this comment.
Safe because there can only be one component per token pairs, right?
There was a problem hiding this comment.
From the contract code, (token0, token1) and (token1, token0) are actually two different pairs, so they should be treated as different components. I already changed this and now only keep the lane_index(base_asset, quote_asset) key.
| // A batch can include repeated updates for the same component. The override value is | ||
| // block-wide, so emit it once per component per transaction. | ||
| if !block_overridden_components.insert(component_id.clone()) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
I'd prefer to emit these whenever there is an update, even if it somehow happens twice in the same block
| // Registry calldata only contains `laneIndex`, not the token pair. `laneIndex` is | ||
| // keccak256(abi.encode(tokenA, tokenB)), so this is a one-way lookup through the lane store | ||
| // populated when the pair was created. |
There was a problem hiding this comment.
Interesting, so updates target specific pairs, right?
3 participants
Summary
This PR adds the FermiSwap integration based on the VM integration plan:
https://propeller-heads.atlassian.net/wiki/spaces/~71202094d3d73818b5447b85231f4bd027b78c/pages/3679518721/FermiSwap+Integration+Plan+VM
There is one extra part compared to the original plan: balance tracking for the
trader_vault.FermiSwap keeps all tokens in the
trader_vault, but we still need balances on each component to calculate TVL. To handle that, I added two stores:store_token_pairs: {key: token, value: list of pairs that use this token}store_token_balances: {key: token, value: absolute token balance in thetrader_vault}With these two stores, when a token balance changes in the
trader_vault, we can update all related components that depend on that token.This PR also emits token balance updates for the
trader_vaultitself, since the VM integration needs to read the vault token balances during simulation.