Harden transfers (SafeERC20) + safe polish; sync docs#2
Merged
Conversation
Code (src/QuantDEX.sol) — no API/signature changes: - SafeERC20 for all six transfers (closes SWC-104: tokens returning false or no bool — e.g. USDT — now revert instead of silently succeeding). - CEI made consistent: addLiquidity now writes effects before pulling tokens, matching swap/removeLiquidity. - DRY: addLiquidity/removeLiquidity use the existing _poolKey helper. - Comment fixes: drop the wrong "SWC-120" oracle reference (SWC-120 is weak randomness; cite samczsun); correct the _sqrt NatSpec — the rounding is NOT a security mechanism, internal reserve accounting is what closes inflation. - Document fee-on-transfer / rebasing tokens as unsupported. Tests: only an unused/dead `is IERC20` reference removed from test doubles (forced by switching the contract to OZ's IERC20 for SafeERC20). No logic changed; all 18 Foundry tests pass (incl. invariant fuzz, 256 runs x 128k calls). Docs synced (SECURITY.md, README.md): remove the sqrt/"O(N²) quadratic" inflation misattribution and the SWC-120 reference; credit internal reserve accounting; mark the unchecked-return/unsafe-erc20 Wake findings FIXED; add fee-on-transfer limitation. docs/index.html needed no change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Analysis CompleteGenerated ECC bundle from 1 commits | Confidence: 50% View Pull Request #3Repository Profile
Changed Files (6)
Top hotspots
Top directories
Analysis Depth Readiness (commit-history, 21%)ECC Tools uses this to decide whether recommendations should stay at commit-history/setup guidance or expand into CI, security, harness, reference-set, AI-routing, and team backlog work.
Reference Set Readiness (1/7, 14%)
Generated Instincts (16)
After merging, import with: Files
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Correctness + safe polish on the AMM (no API/signature changes), then docs synced to match. All 18 Foundry tests pass unchanged (incl. invariant fuzz: 256 runs × 128k calls, 0 reverts).
Code (
src/QuantDEX.sol)falseor no bool (USDT/BNB) now revert instead of silently succeeding. (No signature change.)addLiquiditynow writes effects before pulling tokens, matchingswap/removeLiquidity.addLiquidity/removeLiquidityuse the existing_poolKeyhelper.SWC-120oracle ref (that's weak randomness; cite samczsun); correct the_sqrtNatSpec (the rounding is not a security mechanism — internal reserve accounting is what closes inflation).Tests
Only a dead
is IERC20reference removed from two test doubles (forced by switching the contract to OZ'sIERC20for SafeERC20). No logic changed.Docs (
SECURITY.md,README.md)Remove the
sqrt/"O(N²) quadratic" inflation misattribution and the SWC-120 reference; credit internal reserve accounting; mark the Wakeunchecked-return/unsafe-erc20findings Fixed; add the fee-on-transfer limitation. Matches the blog post.🤖 Generated with Claude Code
Summary by cubic
Hardened ERC‑20 transfers with
@openzeppelin/contractsSafeERC20and alignedaddLiquiditywith CEI; no API/signature changes. Docs now correctly attribute the inflation defense to internal reserve accounting and note that fee‑on‑transfer/rebasing tokens are unsupported.Refactors
SafeERC20(safeTransfer*); tokens returning false or no value now revert (closes SWC‑104).addLiquiditycredits state before interactions;addLiquidity/removeLiquidityuse_poolKey; tests only drop unusedIERC20refs.Dependencies
IERC20andSafeERC20from@openzeppelin/contracts.Written for commit 1985e5f. Summary will update on new commits.