Repository navigation
Release v1.1.0: Pashov solidity-auditor v4, ethskills facts, Uniswap corrections - #10
Merged
ccashwell merged 6 commits intoSep 29, 2026
Merged
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b96043a3d5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+159
to
+160
| indexed=$(wc -l < "$work/index.tsv" | tr -d ' ') | ||
| if [ "$opens" != "$closes" ] || [ "$opens" != "$indexed" ]; then |
There was a problem hiding this comment.
The structure check treats every closed block as readable without validating required attributes such as key, kind, and agents. If the model mistypes kind, the later FINDING/LEAD filters silently discard the block while this check reports no problem; if key or agents is omitted, the empty TSV field is collapsed by Bash and the assembler can abort while trying to read a shifted field as a filename. Validate these attributes and count invalid blocks as unreadable before deduplication.
Useful? React with 👍 / 👎.
Comment on lines
+146
to
+148
| ### Turn 6 — Severity and PoC annex (EVM Cortex, once per scan) | ||
|
|
||
| Without concrete proof, it's a LEAD, not a finding. Leads are honest | ||
| about what you couldn't verify — they're not failures, they're | ||
| calibration. Emit them. | ||
| Runs after Turn 5, at any pass count. It reads `full-report.md` and never writes to it. |
There was a problem hiding this comment.
Turn 5 has already printed and optionally copied full-report.md before this turn runs, and this turn explicitly never updates that report. Consequently, when a Critical/High PoC fails—or the subsequent fix verification rejects the proposed fix—the primary report still presents the original severity and fix; only the separate annex is downgraded, even though the annex says such findings require a working PoC before being reported at that severity. Perform these verification gates before assembly or regenerate the authoritative report from their verdicts.
Useful? React with 👍 / 👎.
…or v4
Upstream (github.com/pashov/skills, 2026-09-23) moved solidity-auditor
from v3 to v4: loop mode, scan memory, and a shell-assembled report.
- Loop mode: `--loop N` runs N passes of the twelve agents in one scan;
every pass after the first is handed what earlier passes found as
"ground already walked", and the scan ends in one combined report.
- Scan memory: `--memory` (automatic when passes > 1) keeps a ledger at
`.solidity-auditor/memory.tsv` in the audited repo. Findings print as
KNOWN (n scans) or NEW; records not raised again are listed under
"Known from earlier scans" and marked not re-checked.
- Shell-assembled report: each pass writes gated findings to a run file
while it still has context, and `references/assemble.sh` is the only
producer of full-report.md. The orchestrator never composes or
re-words the report, so it can never overstate its coverage.
- Findings are worded in Simplified Technical English
(report-language.md, appended to every bundle); confidence threshold
moves from 80 to 75 so promoted leads clear it; deploy scripts are in
scope; agents are READ-ONLY inside the audited repo; the upgrade
warning fires only when the local VERSION is lower than upstream.
Vendoring layout: upstream's SKILL.md is carried verbatim as
`references/orchestration.md` so the turn-by-turn procedure (memory
read, prune, bundle build, run files, assembly) stays a file-by-file
diff. The local SKILL.md is now a wrapper holding only the EVM Cortex
layer: agent mapping, context package, Foundry pre-flight writing under
the scan directory, a structured `**Severity**` line in each finding
block (pasted through by the assembler unchanged), and a Turn 6
severity/PoC annex whose rows are extracted from the assembled report
by awk rather than typed. PoCs live under
`.solidity-auditor/runs/{stamp}/poc/` and run with FOUNDRY_TEST
overridden, never in the project's test/.
Deviations from upstream, all documented in SKILL.md: the severity/PoC
addendum appended to judging.md (its fix-verification threshold now
75), and onchain/offchain spelling in references/ prose plus the one
disclaimer string assemble.sh prints. Everything else under references/
is byte-identical to upstream after that normalization.
Verified: assemble.sh runs end to end on a synthetic three-finding scan,
the severity line survives into full-report.md, and the annex extractor
returns rows equal to the assembler's finding count.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Chris Cashwell <chris.cashwell@uniswap.org>
ethskills.com (fetched 2026-09-25) has moved on from the March copy the rules were written against. Corrections folded into rules/evm-current-state.md: - Fusaka shipped Dec 3, 2025 (PeerDAS, gas limit 30M -> 60M). Glamsterdam is in progress for Q3-Q4 2026 with FOCIL removed from scope; Hegota (Q4 2026) does not carry Verkle, and EIP-7864 (binary state tree) is the Draft candidate. Scheduling status is forkcast CFI/SFI/DFI per EIP-7723, not roadmap diagrams. - Unichain (chain ID 130, TEE time-priority ordering) and Robinhood Chain (chain ID 4663, Orbit L2 whose ArbOS 61 filtering can censor even force-included transactions) were missing from the L2 list. - Aerodrome and Velodrome merged into Aero (Nov 2025); Base announced its Superchain exit (Feb 2026); Celo's stablecoins rebranded to USDm/EURm/BRLm. - Hardhat 3 (Aug 2025) is a legitimate toolchain again. Foundry stays this squad's default; the rule no longer calls Hardhat obsolete. - ERC-8004 registry and ERC-4337 EntryPoint v0.7 addresses added after `cast code` verification on mainnet (and Base for ERC-8004). agents/eip-expert.md listed Fusaka as "targeting late 2026" and Glamsterdam as "2027+"; both corrected, with a Hegota section and a CFI/SFI/DFI guide. agents/l2-specialist.md gains Unichain, Robinhood Chain, chain IDs, and the dominant-DEX-per-chain note. skills/erc8004-patterns replaces its `0x...` placeholder with the verified registry addresses. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Chris Cashwell <chris.cashwell@uniswap.org>
…and docs The seven Uniswap skills and six matching agents were written in April 2026 and never re-checked. Every claim below was verified against v4-core/v4-periphery main, developers.uniswap.org, or onchain state with `cast code`/`cast call` on 2026-09-25. Compile-breaking API drift: - SwapParams and ModifyLiquidityParams live in v4-core/src/types/PoolOperation.sol, not IPoolManager. ~35 uses fixed. - v4-periphery removed src/utils/BaseHook.sol and HookMiner.sol on 2026-02-06 (PR #510). Examples now import @openzeppelin/uniswap-hooks/src/base/BaseHook.sol (pinned by Uniswap/v4-template) or Uniswap/v4-hooks-public, take HookMiner from v4-periphery/test/shared/, and override _beforeSwap/_afterSwap internally instead of the non-virtual external callbacks. - uniswap-v4-testing had the V4 sign convention inverted: negative amountSpecified is exact input. All 22 SwapParams literals, the direction table, and the delta assertions fixed. It also redeclared `PoolKey key` over the Deployers field, forked block 21_000_000 (PoolManager first has code at 21_688_329), used a bad EIP-55 checksum for the PoolManager, and called a 6-arg getPositionLiquidity that does not exist. - agents/uniswap-v4-expert settled ERC-20 deltas without sync(), which reverts CurrencyNotSettled(); native branch added. Factual corrections: - V3 TickLens address was non-hex and pointed at empty code. - V4 positions are ERC-721; ERC-6909 is the PoolManager claim token. - Protocol fees: set by protocolFeeController (not the owner), charged on swap input before the LP fee, lower 12 bits zeroForOne, capped at 1000 pips. sync() is callable outside unlock(). notifyBurn takes five arguments. Actions has 26 codes. 10 hook callbacks, 14 flags. - V4 is on 19 mainnets (Blast dropped; X Layer, Robinhood Chain, Arc added). docs.uniswap.org redirects to developers.uniswap.org. - V3 SwapRouter v1 labelled legacy: absent on Base, Unichain, BNB, Avalanche, Celo. UniversalRouter 0x66a9... is V2; 2.1.1 and 2.1.2 added. `forge install --no-commit` no longer exists. - TickMath.getSqrtPriceAtTick is the V4 name; V3 is getSqrtRatioAtTick. New facts the skills lacked: - The protocol fee switch is live: V3 mainnet since UNIfication executed 2025-12-28 (slot0.feeProtocol 68 on 0.01%/0.05% tiers, 102 on 0.30%), V4 static-fee pools since 2026-07-27 (ETH/USDC 500/10 protocolFee 512125). Fee-revenue formulas now carry (1 - protocol_share). - Unichain (chain ID 130) V3 and V4 addresses; Ethereum StateView, V4Quoter, PositionDescriptor, ReservesLens, Permit2. All checksummed and code-verified on their chain. - pool-finder queried getSlot0 on the PoolManager, which has no such ABI and reverts; it now calls StateView, and its PoolId example reproduces the live ETH/USDC pool id. Numeric fixes recomputed at 80-digit precision: sqrtPriceX96 worked examples (a 10^3 error and float-tail artifacts), tick for 3e-9 (-196257), maxLiquidityPerTick magnitudes (10^5 error), capital-efficiency multipliers (+-10% is 10.5x, not 5.2x), and the sqrtPriceX96 decimals scaling in pool-finder, whose helper truncated to zero for 18/6-decimal pairs. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Chris Cashwell <chris.cashwell@uniswap.org>
CI runs ShellCheck at warning level over every .sh file in the repo, and the vendored assemble.sh binds all thirteen TSV columns by name in its `read` loops while using only some of them. ShellCheck reports the five never-read names (key, kind, agents, start, end) as SC2034. Add a file-level `# shellcheck disable=SC2034` directive rather than renaming the fields to `_`: the named reads document the column order, and a two-line header keeps the rest of the script byte-identical to upstream so the next sync stays a clean diff. Recorded as the fourth documented deviation in SKILL.md. Assembler output on the synthetic scan is unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Chris Cashwell <chris.cashwell@uniswap.org>
The job failed with "Could not fetch an OIDC token. Did you remember to add `id-token: write`?": the ANTHROPIC_API_KEY secret is empty in this repo, so claude-code-action@v1 falls back to OIDC federation, which the workflow's permissions block did not allow. Also replace `direct_prompt` with `prompt` and drop `review_comment_prefix`; the action logged both as unexpected inputs, so even with credentials the review would have run without a prompt. The job still needs either the ANTHROPIC_API_KEY secret or an Anthropic federation rule configured for this repository before it can pass. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Chris Cashwell <chris.cashwell@uniswap.org>
ccashwell
force-pushed
the
claude/skill-refresh-pashov-v4-ethskills-uniswap
branch
from
September 29, 2026 14:14
b96043a to
418326a
Compare
Bump package.json 1.0.2 -> 1.1.0 and bring plugin.json and the CLI's VERSION constant, both left at 1.0.0 through the 1.0.1 and 1.0.2 bumps, up to the same number. Promote the Unreleased section of UPGRADING.md to v1.1.0 (2026-09-29). Minor rather than patch: installers gain --update, four skills are new (simao-audit-pipeline, fizz, fizz-sync, fizz-convert), and pashov-audit-pipeline's report shape, flags, and threshold changed with upstream v4. Nothing removed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Chris Cashwell <chris.cashwell@uniswap.org>
ccashwell
changed the title
ccashwell
deleted the
claude/skill-refresh-pashov-v4-ethskills-uniswap
branch
September 29, 2026 14:32
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
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.
Summary
Freshness pass over the vendored and homegrown skills, three atomic commits:
pashov-audit-pipeline→ solidity-auditor v4 (e65d967). Upstream (2026-09-23) added loop mode (--loop N), a findings ledger across scans (--memory), and a shell-assembled report so the orchestrator can never overstate coverage. Upstream'sSKILL.mdis vendored verbatim asreferences/orchestration.md; the localSKILL.mdis only the EVM Cortex wrapper (agent mapping, context package, Foundry pre-flight, a**Severity**line per finding block, and a Turn 6 severity/PoC annex extracted by shell). Threshold moved 80 → 75 with upstream.fizz,x-ray, andsimao-audit-pipelinewere already byte-identical to their pinned upstream versions.41dfa48).rules/evm-current-state.md,agents/eip-expert.md,agents/l2-specialist.md,skills/erc8004-patternsupdated from ethskills.com as of 2026-09-25: Fusaka shipped, Glamsterdam/Hegota scope, Unichain and Robinhood Chain, Aero merger, Hardhat 3, ERC-8004 and EntryPoint v0.7 addresses (allcast code-verified).b96043a). Seven skills and six agents checked against v4-core/v4-peripherymain, developers.uniswap.org, and onchain state. Compile-breaking fixes:SwapParams/ModifyLiquidityParamsmoved toPoolOperation.sol;BaseHook/HookMinerremoved from v4-periphery in Feb 2026 (now OpenZeppelinuniswap-hookswith internal_beforeSwap-style overrides); the V4 testing skill had the swap sign convention inverted throughout. Fact fixes: TickLens address, V4 positions are ERC-721, protocol-fee mechanics,sync()unlock semantics, 19-mainnet chain list. New: protocol fee switch live on V3 (UNIfication, 2025-12-28) and V4 (2026-07-27); Unichain V3/V4 addresses; StateView-basedcastexamples inpool-finder. Numeric worked examples recomputed at 80-digit precision.UPGRADING.mdrecords all three under Unreleased.Verification
references/diffed against upstream after boundary-safeonchainnormalization: only the documentedjudging.mdaddendum and one disclaimer word inassemble.shdiffer.assemble.shrun end to end on a synthetic three-finding scan; severity line survives intofull-report.md; annex extractor row count equals the assembler's finding count.cast code-checked on its chain and written EIP-55 checksummed. Upstream API claims verified against raw GitHubmain.on-chainspellings in touched files.Not verified
assemble.shgets its first ShellCheck pass in this CI run (warning level, upstream code).Release
This PR is the v1.1.0 release. The final commit bumps
package.json(1.0.2 → 1.1.0) and bringsplugin.jsonand the CLIVERSIONconstant, both stuck at 1.0.0 since the first release, to the same number;UPGRADING.md's Unreleased section is promoted tov1.1.0 (2026-09-29). Minor rather than patch because installers gain--update, four skills are new, and the Pashov pipeline's flags, report shape, and threshold changed.Two follow-ups after merge, both outside this PR:
v1.1.0(already created with notes; publishing creates the tag onmain).npm publishfrommain, since 1.0.2 is the version on the registry.CI fixes bundled here: ShellCheck directive in the vendored
assemble.sh, DCO sign-offs on every commit, andid-token: writeplus current action inputs for the claude-review workflow. That job now passes but runs no review untilANTHROPIC_API_KEYor an Anthropic federation rule is configured.🤖 Generated with Claude Code