Skip to content

feat: add slippage floor validation for RocketPool STAKE and SWAP#21

Closed
ajag408 wants to merge 6 commits into
mainfrom
feat/eng-2460-rocketpool-slippage-floor
Closed

feat: add slippage floor validation for RocketPool STAKE and SWAP#21
ajag408 wants to merge 6 commits into
mainfrom
feat/eng-2460-rocketpool-slippage-floor

Conversation

@ajag408
Copy link
Copy Markdown
Contributor

@ajag408 ajag408 commented May 16, 2026

Summary by CodeRabbit

  • New Features

    • Added slippage protection for swap transactions, enforcing an 80% minimum output floor to prevent unfavorable price outcomes.
  • Tests

    • Expanded test coverage for swap validation and slippage-floor enforcement across multiple swap variants.
  • Chores

    • Enhanced CI/release workflows with runner hardening and firewall security controls.
    • Updated pnpm installation approach and npm configuration for improved supply-chain security.

Review Change Stack

  • Adds input-relative slippage floor (80%) for minTokensOut on STAKE and _minAmountOut on SWAP in the RocketPool validator
  • Previously, Shield only checked minTokensOut > 0 (STAKE) and validated _receiver (SWAP) — a compromised upstream could set minTokensOut = 1 wei on a 1 ETH stake and pass validation, exposing the transaction to MEV sandwich extraction
  • The 80% floor is conservative — rETH has never traded below 0.90 ETH/rETH — no risk of blocking legitimate transactions
  • Self-contained in the RocketPool validator: no API changes, no monorepo impact, no cross-cutting dependencies

What changed

File Change
src/validators/evm/rocketpool/rocketpool.validator.ts Added SLIPPAGE_FLOOR_BPS constant, slippage check in validateStake, _minAmountOut > 0 + slippage check in validateLifiSwapReceiver (with Single/Multiple variant handling)
src/validators/evm/rocketpool/rocketpool.validator.test.ts Added 16 new tests: STAKE slippage boundary tests, SWAP slippage tests (zero, manipulated, boundary), Multiple variant coverage (happy path, receiver, Permit2 wrapping, slippage), multi-entry swapData summing

Security context

Addresses the slippage parameter manipulation gap identified in the Shield security review. Unlike the args.amount validation (Phase 6), this fix is independently shippable because both the input (tx.value / fromAmount) and output floor (minTokensOut / _minAmountOut) are already present in the transaction calldata.

Threat model: requires a compromised upstream API (consistent with Shield's threat model). The validator remains fail-closed — unknown LI.FI function selectors are blocked, not bypassed.

Test plan

  • All 67 RocketPool tests pass (59 existing + 8 new slippage + 8 new Multiple variant + 1 new general)
  • Existing valid transactions unaffected (use 90% slippage, well above 80% floor)
  • Manipulated slippage (1 wei) blocked for both STAKE and SWAP
  • Zero _minAmountOut blocked on SWAP
  • Boundary tests: exactly-at-80% passes, 80%-minus-1-wei blocked
  • Permit2-wrapped SWAP with manipulated slippage blocked (all 3 proxy methods)
  • Multiple variant coverage: happy path, wrong receiver, multi-entry fromAmount summing, slippage floor
  • Full test suite green (9/10 suites pass; Tron failure is pre-existing on main)

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 16, 2026

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

This PR hardens the project's supply chain and CI/CD pipelines by adding runner hardening and Socket firewall checks across all GitHub Actions jobs, modernizing pnpm installation via corepack, and updating code ownership rules. Additionally, it introduces an 80% slippage-floor validation constraint in RocketPoolValidator to protect against low-slippage STAKE and LI.FI swaps, backed by comprehensive test coverage.

Changes

Supply-Chain & CI/CD Hardening

Layer / File(s) Summary
Code ownership and npm configuration
.github/CODEOWNERS, .npmrc, package.json
Code owners assigned to supply-chain critical files (.npmrc, package.json, pnpm-lock.yaml). npm pre/post-script execution disabled; minimum release age set to 4320 minutes. pnpm restricted to only build esbuild as a native dependency.
CI workflow hardening and pnpm modernization
.github/workflows/ci.yml
All test, security, and dependency-review jobs add step-security/harden-runner and socketdev/action firewall steps. Checkout updated with persist-credentials: false. pnpm/action-setup replaced with corepack-based pnpm 10.12.2 provisioning. Codecov input renamed from file to files. Audit and outdated steps set continue-on-error: true. dependency-review action pinned to v4.
Release workflow hardening and modernization
.github/workflows/release.yml
security-audit, publish, build-binaries, and create-release jobs add runner hardening and secure checkout configuration. pnpm installation unified on corepack prepare pnpm@10.12.2. Socket firewall added to all jobs. build-binaries pnpm store caching removed. publish adds npm upgrade and NPM_CONFIG_PROVENANCE: "true".

RocketPoolValidator Slippage-Floor Protection

Layer / File(s) Summary
Slippage-floor validation logic
src/validators/evm/rocketpool/rocketpool.validator.ts
New SLIPPAGE_FLOOR_BPS constant (8000n = 80%) enforces minimum acceptable slippage. validateStake rejects STAKE swaps when minTokensOut falls below 80% of tx.value. validateLifiSwapReceiver rejects LI.FI swaps when _minAmountOut falls below 80% of extracted fromAmount (summed across Multiple swap variants).
Slippage-floor test coverage
src/validators/evm/rocketpool/rocketpool.validator.test.ts
LI.FI swap ABI extended with Multiple variant function signatures. STAKE tests validate rejection on heavy minTokensOut manipulation, acceptance at exactly 80% threshold, and rejection just below floor. SWAP tests add happy-path coverage for Multiple variants (Native/ERC20), Permit2-wrapped variants, and multiple swapData entries; rejection tests verify mismatched receiver detection; slippage-floor tests comprehensively cover rejection on manipulated _minAmountOut, acceptance at threshold, rejection below floor, and summed fromAmount aggregation across Multiple swapData.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • stakekit/shield#13: Extends existing RocketPoolValidator STAKE and LI.FI receiver validation logic, building directly on validator introduced in #13.
  • stakekit/shield#10: Modifies same supply-chain areas (CODEOWNERS, CI/release workflows, pnpm install/audit steps) related to workflow hardening.

Suggested reviewers

  • jdomingos
  • raiseerco
  • Philippoes
  • petar-omni

Poem

🐰 Hops through actions with hardened stride,
Pnpm flows through corepack wide,
Slippage floors now guard the swaps,
Socket firewalls close all gaps,
Supply chain glows, so safe and bright!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding slippage floor validation for RocketPool STAKE and SWAP operations, which is the primary purpose of the pull request.
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.

✏️ 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 feat/eng-2460-rocketpool-slippage-floor

Comment @coderabbitai help to get the list of available commands and usage tips.

@ajag408 ajag408 closed this May 16, 2026
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