feat: validate slippage range in buy and sell commands

[UnknownHacker9991]

Closes #1

Added slippage validation in both buy() and sell() in src/pumpfun_cli/commands/trade.py, right after _validate_mint(). Values outside 0-100 now exit with a clear error message.

Changes:

• src/pumpfun_cli/commands/trade.py — validation in buy() and sell()
• tests/test_commands/test_trade_cmd.py — 6 new tests covering negative, above-100, and boundary values (0, 100)

All 27 tests pass: uv run pytest tests/test_commands/test_trade_cmd.py -v

Summary by CodeRabbit

• Bug Fixes

  • Added slippage validation for buy and sell commands (allowed range 0–100). Out-of-range values now produce a clear error before any wallet or network actions.

• Tests

  • Added comprehensive tests for slippage handling on buy and sell flows, including boundary cases (0 and 100), invalid values (<0, >100), dry-run behavior, and JSON output correctness.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds slippage bounds validation (0–100) to buy and sell commands, invoked before RPC/wallet initialization, and adds tests for out-of-range and boundary slippage cases including JSON and dry-run scenarios.

Changes

Cohort / File(s)	Summary
Slippage Validation src/pumpfun_cli/commands/trade.py	Added private _validate_slippage(slippage: int) -> None and invoked it in both buy and sell flows to error when slippage < 0 or > 100; validation runs before RPC/keyfile/password retrieval.
Slippage Validation Tests tests/test_commands/test_trade_cmd.py	Added tests covering negative and >100 slippage (expect errors), boundary cases 0 and 100 (valid), plus JSON-output and dry-run variations that exercise buy/sell code paths with mocks and wallet setup.

Sequence Diagram(s)

(omitted — change is a small validation addition and tests, not a multi-component flow that requires visualization)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

• #1: Validate slippage range (0-100) in buy and sell commands — This PR implements the requested slippage bounds check and error messaging, addressing the issue objective.
• chainstacklabs/pumpfun-cli#2 — Related: also adds argument validation in src/pumpfun_cli/commands/trade.py (buy-amount validation); both PRs target input validation though this PR focuses on slippage only.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	❓ Inconclusive	The description covers key changes and test results but is missing required template sections like affected layers and transaction construction impact.	Fill out the Layers touched section (commands/ applies) and clarify whether slippage validation affects transaction construction.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: adding slippage validation to buy and sell commands.
Linked Issues check	✅ Passed	Code changes fully implement issue #1 requirements: slippage validation in buy/sell after _validate_mint() with 0-100 range enforcement and comprehensive test coverage.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to issue #1: slippage validation logic, test cases, and no unrelated modifications detected.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

📝 Coding Plan

• Generate coding plan for human review comments

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/pumpfun_cli/commands/trade.py`:
- Around line 79-80: Extract the duplicated slippage check into a shared helper
function (e.g., validate_slippage) and call it from both buy() and sell();
specifically, move the logic that currently checks "if slippage < 0 or slippage
> 100: error(...)" into validate_slippage(slippage) which performs the same
validation and error(...) call, then replace the two in-line guards in buy() and
sell() with a single call to validate_slippage(slippage); ensure the helper is
placed in the same module (or imported) so both functions reference the same
symbol and keep the exact error message/hint behavior.

In `@tests/test_commands/test_trade_cmd.py`:
- Around line 482-544: Add two mirror tests for sell slippage boundaries like
test_buy_slippage_zero and test_buy_slippage_100: create test_sell_slippage_zero
and test_sell_slippage_100 that set XDG_CONFIG_HOME and PUMPFUN_PASSWORD,
encrypt a Keypair into config_dir/wallet.enc (reuse Keypair and
encrypt_keypair), patch pumpfun_cli.commands.trade.sell_token with AsyncMock
returning a buy-like result (action "sell", mint _FAKE_MINT,
sol_spent/tokens_received/signature/explorer), invoke the CLI with
runner.invoke(app, ["--json","--rpc","http://rpc","sell","--slippage","0",
_FAKE_MINT, "0.01"]) and likewise with "--slippage","100", and assert
result.exit_code == 0 and that "Slippage must be between" is not in
result.output; ensure RPC/network is mocked via the AsyncMock patch of
sell_token.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 134cd9ff-5150-4dfd-846e-e6e3ef2894b3

📥 Commits

Reviewing files that changed from the base of the PR and between 8934055 and ebd66a3.

📒 Files selected for processing (2)

• src/pumpfun_cli/commands/trade.py
• tests/test_commands/test_trade_cmd.py

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (1)

tests/test_commands/test_trade_cmd.py (1)

482-544: 🛠️ Refactor suggestion | 🟠 Major

Sell boundary tests (slippage=0, slippage=100) still missing.

Buy boundaries are covered, but sell boundaries are not. Add test_sell_slippage_zero and test_sell_slippage_100 for parity — same pattern as the buy tests, but invoking the sell command.

,

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/test_commands/test_trade_cmd.py` around lines 482 - 544, Add two mirror
tests named test_sell_slippage_zero and test_sell_slippage_100 following the
exact pattern of test_buy_slippage_zero and test_buy_slippage_100: set
XDG_CONFIG_HOME and PUMPFUN_PASSWORD, create Keypair and encrypt_keypair into
config_dir / "wallet.enc", patch pumpfun_cli.commands.trade.sell_token with
AsyncMock to return the same fake result dict, invoke the CLI with ["--json",
"--rpc", "http://rpc", "sell", "--slippage", "0", _FAKE_MINT, "0.01"] (and
"--slippage", "100" for the other), and assert result.exit_code == 0 and that
"Slippage must be between" not in result.output to match the buy boundary tests.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/pumpfun_cli/commands/trade.py`:
- Around line 46-50: Add an explicit return type annotation to the
_validate_slippage function to satisfy linting (Ruff) and maintain consistency:
change the signature of _validate_slippage to include "-> None" (i.e., def
_validate_slippage(slippage: int) -> None:), leaving the body and error() call
unchanged so behavior remains the same.

---

Duplicate comments:
In `@tests/test_commands/test_trade_cmd.py`:
- Around line 482-544: Add two mirror tests named test_sell_slippage_zero and
test_sell_slippage_100 following the exact pattern of test_buy_slippage_zero and
test_buy_slippage_100: set XDG_CONFIG_HOME and PUMPFUN_PASSWORD, create Keypair
and encrypt_keypair into config_dir / "wallet.enc", patch
pumpfun_cli.commands.trade.sell_token with AsyncMock to return the same fake
result dict, invoke the CLI with ["--json", "--rpc", "http://rpc", "sell",
"--slippage", "0", _FAKE_MINT, "0.01"] (and "--slippage", "100" for the other),
and assert result.exit_code == 0 and that "Slippage must be between" not in
result.output to match the buy boundary tests.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 81d01da2-692f-4777-8f5e-4aede9084215

📥 Commits

Reviewing files that changed from the base of the PR and between ebd66a3 and 8103367.

📒 Files selected for processing (2)

• src/pumpfun_cli/commands/trade.py
• tests/test_commands/test_trade_cmd.py

[coderabbitai]

♻️ Duplicate comments (1)

src/pumpfun_cli/commands/trade.py (1)

46-50: ⚠️ Potential issue | 🟡 Minor

Add explicit -> None on _validate_slippage.

_validate_slippage still triggers ANN202; add the return type to keep lint clean and typing consistent.

♻️ Suggested patch

-def _validate_slippage(slippage: int):
+def _validate_slippage(slippage: int) -> None:
     """Validate slippage is between 0 and 100, or exit with error."""
     if slippage < 0 or slippage > 100:
         error("Slippage must be between 0 and 100.", hint=f"Got: {slippage}")

#!/bin/bash
# Verify no unannotated _validate_slippage signature remains.
rg -nP '^def _validate_slippage\(slippage: int\)(?!\s*->)' src/pumpfun_cli/commands/trade.py

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/pumpfun_cli/commands/trade.py` around lines 46 - 50, The function
_validate_slippage currently lacks an explicit return type and triggers ANN202;
update its signature to include an explicit -> None return annotation (i.e., def
_validate_slippage(slippage: int) -> None:) and keep the body unchanged so the
linter recognizes it as intentionally returning nothing; ensure the change is
applied to the _validate_slippage definition in trade.py so the ripgrep check
provided in the review will pass.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@src/pumpfun_cli/commands/trade.py`:
- Around line 46-50: The function _validate_slippage currently lacks an explicit
return type and triggers ANN202; update its signature to include an explicit ->
None return annotation (i.e., def _validate_slippage(slippage: int) -> None:)
and keep the body unchanged so the linter recognizes it as intentionally
returning nothing; ensure the change is applied to the _validate_slippage
definition in trade.py so the ripgrep check provided in the review will pass.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 04d555c9-f17f-4b7c-bb42-09b25cca9c34

📥 Commits

Reviewing files that changed from the base of the PR and between 8103367 and e846095.

📒 Files selected for processing (2)

• src/pumpfun_cli/commands/trade.py
• tests/test_commands/test_trade_cmd.py

[UnknownHacker9991]

Added -> None return annotation to _validate_slippage to fix ANN202. All 29 tests pass, amended and force-pushed.

[smypmsa]

@UnknownHacker9991 please run:

uv run ruff format tests/test_commands/test_trade_cmd.py

Then commit and push.

[UnknownHacker9991]

Done — formatted with ruff and force-pushed.