feat(erc4626): accept zero-amount transactions instead of blocking (ENG-2372)

[ajag408]

Summary

Shield now accepts zero-amount ERC-4626 transactions for recognized operation
patterns instead of rejecting them. Previously, a zero-amount deposit / mint /
withdraw / redeem / unwrap / wrap (tx.value === 0) was hard-blocked
("… amount is zero" / "WRAP transaction must send ETH value"), which surfaced
to clients as the aggregate "No matching operation pattern found" error — the
root cause of the redeem(0) false blocks seen in production soak (ENG-2372).

Why this approach: we don't know why clients construct zero-amount txs and
don't want to break unknown flows. A zero-amount transaction is a no-op with no
visible security risk, so blocking it adds friction without protection. This
supersedes the earlier plan to reject amount <= 0 upstream in the monorepo with
a 412.

Scope is amount-only. Acceptance does not loosen any other invariant. A
zero-amount tx still must pass every existing check — whitelisted vault, correct
selector, no stray ETH value, and owner / receiver must equal the
user / expected address. Declared-amount intent matching from main is unchanged:
if args.amount / args.shareAmount is present and does not match calldata, the
tx is still blocked (including wrap value === 0 vs a non-zero declared amount).
Only the amount === 0 / value === 0 short-circuit reject is removed.
UNWRAP still has no numeric declared-amount check — that is main's design
(WETH-exit follow-up; validateUnwrap(tx, chainId) only). This PR does not
change that.

Telemetry. When an accepted transaction has a zero amount, the result now
carries a non-blocking details.flags: ['ZERO_AMOUNT'] signal so we can observe
how often and why clients send these, and revisit a targeted block later if the
data warrants it. Consuming this flag in monorepo ShieldValidationService logs
is a follow-up (needs the @yieldxyz/shield bump).

Changes

• src/types/index.ts — add optional details.flags?: string[] to ValidationResult.
• src/validators/base.validator.ts — safe(flags?) can attach non-blocking flags.
• src/validators/evm/erc4626/erc4626.validator.ts — remove the zero-amount blocks
  in validateSupply, validateWithdraw, validateUnwrap, and validateWrap;
  emit ZERO_AMOUNT when the amount / wrap value is zero. Receiver, owner, and
  declared-amount intent checks from main still run.
• src/validators/evm/erc4626/erc4626.validator.test.ts — flip zero-amount
  reject-tests (2-arg deposit, Sky 3-arg deposit, withdraw, unwrap, wrap) to
  assert acceptance + flag; add guard tests (non-whitelisted vault, receiver
  mismatch, owner mismatch); add wrap value === 0 vs declared amount cases.
• package.json — 1.5.0 → 1.6.0 (backward-compatible behavior change on top
  of current main).
  approve(0) is unchanged — the approval validator already allows it (USDT
  zero-allowance reset) and skips intent matching for zero approvals. That is the
  precedent this change follows.

QA Proof

Full Shield test suite green after merging main, with flipped zero-amount
acceptance cases, amount-only guard tests, and wrap declared-amount interaction
tests:

What Needs to Be QA'd in Staging

• Deploy with SHIELD_MODE=MONITOR; send a zero-amount redeem(0) /
  withdraw(0) on a whitelisted ERC-4626 yield and confirm logs now show
  Shield validation successful (was Shield validation failed /
  No matching operation pattern).
• Confirm the ZERO_AMOUNT flag is present on the accepted result (once
  this package is bumped in the monorepo; until then the tx should at least
  stop failing Shield).
• Native-ETH WETH vault: wrap with value = 0 is accepted (same flag),
  unless args.amount is a non-zero declared intent — that must still block.
• Regression: a zero-amount tx with a tampered owner / receiver (not the
  user) is still blocked.
• Regression: normal non-zero deposit / withdraw / redeem still validate as
  before (flags absent on the result).
• Regression: declared-amount intent matching still holds (calldata ≠
  args.amount is still blocked).
• Zero-amount tx to a non-whitelisted vault is still blocked.

QA Team Notification

• QA team has been notified to test in staging.

---

Note

Medium Risk
Changes transaction-validation outcomes for zero-amount ERC-4626 flows that integrators may rely on for blocking; security invariants besides amount are unchanged but behavior is user-visible in Shield.

Overview
ERC-4626 validation no longer hard-fails on zero amounts for wrap, supply, withdraw, and unwrap when all other checks pass (whitelisted vault, correct selectors, receiver/owner, declared intent, etc.). Accepted zero-amount txs return isValid: true with a non-blocking details.flags entry ZERO_AMOUNT for observability.

API surface: ValidationResult.details.flags is added, and BaseValidator.safe() can attach optional flags. Package version is bumped to 1.6.0.

Tests are updated from reject-to-accept for zero amounts and add regressions proving malicious zero-amount txs (wrong vault, receiver, owner) still block.

^{Reviewed by Cursor Bugbot for commit 34fd645. Configure here.}

[coderabbitai]

📝 Walkthrough

Walkthrough

ValidationResult gains an optional flags?: string[] field for non-blocking signals. BaseValidator.safe() is updated to accept and forward optional flags. ERC4626Validator changes WRAP, SUPPLY, WITHDRAW, and UNWRAP zero-amount guards from blocking rejections to safe(['ZERO_AMOUNT']) returns. Tests are updated accordingly and the package version is bumped to 1.3.1.

Changes

ZERO_AMOUNT Non-Blocking Flag

Layer / File(s)	Summary
ValidationResult flags field and BaseValidator.safe update src/types/index.ts, src/validators/base.validator.ts	ValidationResult gains optional flags?: string[]; BaseValidator.safe() accepts optional flags and includes them in the details payload only when non-empty.
ERC4626 zero-amount checks converted to ZERO_AMOUNT flag src/validators/evm/erc4626/erc4626.validator.ts	validateWrap, validateSupply, validateWithdraw, and validateUnwrap each replace their zero-amount blocked return with safe(['ZERO_AMOUNT']).
ERC4626 tests and version bump src/validators/evm/erc4626/erc4626.validator.test.ts, package.json	WRAP, SUPPLY, WITHDRAW, and UNWRAP test cases switch from rejection assertions to acceptance assertions with ZERO_AMOUNT flag checks; negative coverage for unwhitelisted vault/wrong receiver/wrong owner is retained. Version bumped to 1.3.1.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• stakekit/shield#19: Modifies the same erc4626.validator.ts SUPPLY/WITHDRAW paths and adjusts zero-amount and receiver logic.

Suggested reviewers

• jdomingos
• Philippoes
• auralshin
• aditya172926

Poem

🐇 Hop hop, zero's no foe,
A flag now blooms where blocks used to grow,
ZERO_AMOUNT waves a gentle sign,
No hard stop — just a warning in line.
The rabbit sees safe returns shine! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly matches the main change: ERC4626 now accepts zero-amount transactions and flags them instead of blocking them.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch eng-2372-accept-zero-amount-erc4626

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/validators/evm/erc4626/erc4626.validator.ts (1)

234-254: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Stale comment contradicts new behavior.

The comment // WRAP must send ETH value no longer holds—zero-value wraps are now accepted and flagged as ZERO_AMOUNT. Update it to avoid misleading future readers of this security validator.

📝 Suggested comment update

-    // WRAP must send ETH value
+    // WRAP ETH value (zero is accepted and flagged as ZERO_AMOUNT)
     const value = BigInt(tx.value ?? '0');

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/validators/evm/erc4626/erc4626.validator.ts` around lines 234 - 254, The
inline comment above the wrap validation in erc4626.validator.ts is stale and
contradicts the current ZERO_AMOUNT handling. Update the comment near the
`value` check and `parseAndValidateCalldata` logic to reflect that zero-value
wraps are accepted and reported as `ZERO_AMOUNT`, so future readers are not
misled about the `deposit` flow in `ERC4626Validator`.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/validators/evm/erc4626/erc4626.validator.ts`:
- Around line 234-254: The inline comment above the wrap validation in
erc4626.validator.ts is stale and contradicts the current ZERO_AMOUNT handling.
Update the comment near the `value` check and `parseAndValidateCalldata` logic
to reflect that zero-value wraps are accepted and reported as `ZERO_AMOUNT`, so
future readers are not misled about the `deposit` flow in `ERC4626Validator`.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0109fc18-f89b-4f20-817c-e58026189c7f

📥 Commits

Reviewing files that changed from the base of the PR and between bb1ee30 and 150a4a6.

📒 Files selected for processing (5)

• package.json
• src/types/index.ts
• src/validators/base.validator.ts
• src/validators/evm/erc4626/erc4626.validator.test.ts
• src/validators/evm/erc4626/erc4626.validator.ts

[cursor]

Risk: medium. Left a non-blocking comment and did not approve: this PR changes Shield ERC-4626 validation from blocking to accepting zero-amount wrap/supply/withdraw/unwrap, which is above the low-risk approval threshold. Human review is needed; no additional reviewers assigned because reviewers are already requested. Bugbot and Security Agent completed successfully with no findings requiring attention.

  

_{Sent by Cursor Approval Agent: Pull Request Router and Approver}