Skip to content

feat(tools): fynd-tools-common shared crate + audit tidy (ENG-6121)#262

Open
kayibal wants to merge 5 commits into
mainfrom
feat/eng-6121-tools-common
Open

feat(tools): fynd-tools-common shared crate + audit tidy (ENG-6121)#262
kayibal wants to merge 5 commits into
mainfrom
feat/eng-6121-tools-common

Conversation

@kayibal

@kayibal kayibal commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Foundational shared crate fynd-tools-common (audit + Hindsight helpers), erc20-overrides tidy, and docker dependency-cache fix. Base of the Hindsight stack (ENG-6120). Stacked PR #1/6; base main.

Comment on lines +47 to +52
const OZ_V5_BALANCES_NS: B256 =
B256::new(alloy::hex!("52c63247e1f47db19d5ce0460030c497f067ca4cebf71ba98eeadabe20bace00"));

/// OZ v5 `_allowances` is field 1 in `ERC20Storage`, so namespace + 1.
const OZ_V5_ALLOWANCES_NS: B256 =
B256::new(alloy::hex!("52c63247e1f47db19d5ce0460030c497f067ca4cebf71ba98eeadabe20bace01"));

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 OZ v5 namespace constants are unverified by their derivation. OZ_V5_BALANCES_NS/OZ_V5_ALLOWANCES_NS are hardcoded precomputed keccak values. The two tests (oz_v5_*_differs_from_standard) only assert these differ from the standard mapping slots — they pass even if a constant is transcribed wrong. A typo would silently make OZ v5 detection miss and fall through to bail!, with no test catching it. Add a test that recomputes keccak256(abi.encode(uint256(keccak256("openzeppelin.storage.ERC20")) - 1)) & ~bytes32(uint256(0xff)) and asserts equality, locking the constant to its documented formula.

@kayibal kayibal force-pushed the feat/eng-6121-tools-common branch from a3db0f2 to e6507a6 Compare June 30, 2026 12:08
@kayibal kayibal closed this Jun 30, 2026
@kayibal kayibal reopened this Jun 30, 2026
kayibal and others added 5 commits June 30, 2026 13:25
find_balance_slot/find_allowance_slot now return the full B256 storage
slot instead of a u64 mapping position, and fall through to the
OpenZeppelin v5 namespaced layout when the standard probe fails. This
makes erc20-overrides the single canonical slot detector and lets callers
write the slot directly without recomputing it from a position.

Migrate fynd-gas-audit and fynd-swap-cli to consume the returned slot.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Introduce fynd-tools-common, a workspace crate holding the pieces the
audit subcommand and the upcoming Hindsight tool both need:

- aggregator: the AggregatorClient trait and AggregatorQuote model
- bps: raw, gas-adjusted, and eth_call basis-point diff math
- eth_call: EthCallRunner, which re-executes encoded calldata on-chain
  via eth_simulateV1 (with an eth_call fallback) to measure real output
  and gas
- fynd: FyndAggregator and make_quote_params

Covered by unit tests for the pure logic plus a gated fork integration
test for EthCallRunner. ERC-20 slot detection is reused from
erc20-overrides.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Move EthCallRunner, the aggregator quote model, BPS math, and the
FyndAggregator wrapper out of the audit subcommand and into
fynd-tools-common. Delete the duplicated erc20.rs and rely on
erc20-overrides via the shared EthCallRunner.

Split the monolithic audit.rs into focused modules (args, output,
execute, blocks, summary, mod) and break the oversized run,
execute_trade, and print_summary functions into smaller helpers.
aggregator.rs now holds only the external clients (Nordstern, KyberSwap,
0x). Behaviour is unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Dockerfile enumerates each workspace member to pre-build dependencies;
the new fynd-tools-common crate was missing, so the workspace manifest
failed to load. Add tools/common to the COPY, stub-source, and cleanup
steps in both build stages.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Restore the public visibility of MAX_PROBE_SLOT so the API breaking-change
check passes; it is a harmless, useful constant for a slot-detection
library. Also rewrap an over-long doc comment in the fork integration
test to satisfy rustfmt.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@kayibal kayibal force-pushed the feat/eng-6121-tools-common branch from e6507a6 to 8fe771d Compare June 30, 2026 12:29
@github-actions

Copy link
Copy Markdown

No API Breaking Changes Detected

The PR title signals breaking changes, but cargo-semver-checks found none.
If the breaking change is behavioral, CLI, or config-level (not public Rust API), this is expected.
Otherwise, consider using fix: instead of feat: in the PR title.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant