feat: benchmark and devnet skills#164
Conversation
zjg555543
left a comment
There was a problem hiding this comment.
Code Review — PR #164: feat: benchmark and devnet skills
Reviewed all 4 new SKILL.md files against the actual codebase. Found 3 Medium and 2 Minor issues — all are factual inaccuracies in the skill definitions that could mislead the AI assistant.
Medium Issues
1. Adventure SKILL.md: init amount is 10ETH, should be 10000ETH
All 5 "equivalent to" command blocks use 10ETH, but tools/adventure/Makefile:5 defines INIT_AMOUNT := 10000ETH. This is a 1000x difference — if the AI follows the manual commands instead of make, init will fail due to insufficient funds.
Affected lines: 23, 32, 41, 50, 59 in .claude/skills/adventure/SKILL.md
Note:
tools/adventure/README.mdhas the same error — the SKILL.md likely copied from it.
2. Adventure SKILL.md: wrong accounts file path in preconditions
Precondition says testdata/accounts-20k.txt must be present, but:
- Actual path is
testdata/accounts/accounts-20000.txt(note theaccounts/subdirectory and20000vs20k) - The file is auto-generated by
EnsureAccountsFile()inbench/common.go:16, so it's not a real precondition - The SKILL.md itself correctly describes auto-generation in Section 2 (
testdata/accounts/accounts-<N>.txt), contradicting its own precondition
Affected line: 67 in .claude/skills/adventure/SKILL.md
3. Devnet SKILL.md: clean.sh does NOT sync example.env → .env
The skill says "edit example.env and run ./clean.sh to sync" (Section 0, line 14) and ./clean.sh # sync example.env -> .env (Section 2, line 51). But devnet/clean.sh:17 guards the copy with if [ ! -f "$target" ] — it only creates .env when .env does not already exist. An existing .env is never overwritten.
Following this instruction will silently fail to propagate changes.
Suggestion: Change to "delete .env first, then run ./clean.sh to recreate it from example.env" or simply "cp example.env .env".
Minor Issues
4. Devnet SKILL.md: only mentions op-geth-seq, but default SEQ_TYPE is reth
Section 4 says "Port 8123 is the op-geth-seq / sequencer RPC", but example.env:84 defaults to SEQ_TYPE=reth, meaning port 8123 is actually served by op-reth-seq. The debug command in Section 4.1 (docker compose logs --tail=200 op-geth-seq) would also point at the wrong container.
Suggestion: Use op-geth-seq or op-reth-seq (depending on SEQ_TYPE).
5. SA-Benchmark SKILL.md: xlayer-seq container doesn't exist in devnet
Section 3 references docker logs xlayer-seq, but the devnet uses op-geth-seq / op-reth-seq. When targeting the local devnet, this command would fail.
Affected line: 106 in .claude/skills/sa-benchmark/SKILL.md
Overall the skill definitions are well-structured with good security practices (sensitive data masking, clear error boundaries). The issues above are localized factual inaccuracies, mostly from copying the outdated tools/adventure/README.md.
Done.
Done, make testdata/accounts/accounts-20000.txt as default.
letf user define .env, maybe from example.env.
op-geth is deprecated. So just use op-reth-seq.
change to op-reth-seq as default.
|
No description provided.