fix: align instruction builders with IDL signer/writable flags and data encoding#18
Merged
fix: align instruction builders with IDL signer/writable flags and data encoding#18
Conversation
…ta encoding Audited all instruction builders against pump_fun_idl.json and pump_fun_amm_idl.json. Fixed 8 discrepancies: OptionBool encoding (1 byte not 2), removed spurious track_volume from bonding curve buy/sell, corrected writable/signer flags on create_v2 and extend_account, fixed claim_cashback signer flag, and added missing PUMP_AMM_PROGRAM account to collect_coin_creator_fee. E2e mainnet tests confirm PumpSwap buy+sell, bonding curve buy+sell, and token launch all work correctly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughRefactors instruction payloads (track-volume encoding moved between bonding-curve and PumpSwap builders), adjusts account signer/writable flags, updates unit tests, and enhances mainnet test script with PumpSwap candidate iteration, slippage control, and a new token-launch workflow that generates a temporary PNG and exercises buy/sell flows. Changes
Sequence Diagram(s)sequenceDiagram
actor CLI as pumpfun-cli
participant BC as Bonding Curve
participant AMM as PumpSwap AMM
rect rgba(100,150,200,0.5)
Note over CLI,BC: Bonding-curve trades use --slippage 50 and exclude track-volume
CLI->>BC: buy (slippage 50)
BC-->>CLI: Success / Fail
CLI->>BC: sell (slippage 50)
BC-->>CLI: Success / Fail
end
rect rgba(150,200,100,0.5)
Note over CLI,AMM: PumpSwap AMM candidate iteration (track-volume appended)
loop for each graduated mint
CLI->>AMM: buy --force-amm --slippage 50
AMM-->>CLI: Error 6023?
alt Error 6023
CLI->>CLI: continue to next candidate
else Other error
CLI->>CLI: record FAIL, stop
else Success
CLI->>AMM: wait 5s then sell all --slippage 50
AMM-->>CLI: Success / Fail
alt Sell fails
CLI->>AMM: retry sell after 5s
end
CLI->>CLI: record PASS, exit loop
end
end
end
sequenceDiagram
actor CLI as pumpfun-cli
participant PIL as PIL (Image Gen)
participant Launch as pumpfun launch
participant BC as Bonding Curve
rect rgba(200,150,100,0.5)
Note over CLI,Launch: Group 6b token launch workflow
CLI->>PIL: generate temp PNG
PIL-->>CLI: temp_image_path
CLI->>Launch: launch --json [--image temp_image_path]
Launch-->>CLI: JSON output (mint)
CLI->>Launch: launch --json --buy 0.001
Launch-->>CLI: Success / Fail (mint)
alt buy success
CLI->>BC: sell all --slippage 50
BC-->>CLI: Success / Fail
alt sell fails
CLI->>BC: retry sell after delay
end
end
CLI->>CLI: persist $LAST_OUTPUT_FILE
CLI->>PIL: remove temp image
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/mainnet-test.sh`:
- Around line 375-378: The script currently treats the launch as a PASS based
only on LAUNCH_EXIT and then later extracts LAUNCHED_MINT from LAUNCH_OUT;
change the logic in the launch handling (references: LAUNCH_EXIT, LAUNCH_OUT,
LAUNCHED_MINT, and record) to validate the parsed JSON/mint before recording
success: after computing LAUNCHED_MINT, check it is non-empty/valid JSON; if
valid, call record "Launch" "launch" "PASS" and continue, otherwise call record
"Launch" "launch" "FAIL" (and log LAUNCH_OUT or the JSON error) and fail the
step so downstream --buy/--sell logic does not assume a mint; apply the same
validation to the other launch branch that sets LAUNCHED_MINT.
- Around line 316-357: The code incorrectly records "All graduated pools hit
error 6023" when a non-6023 AMM failure occurred because AMM_BUY_OK stays false
and AMM_MINT stays empty; fix by introducing a failure marker (e.g.
AMM_NON_OVERFLOW_FAIL) or set AMM_BUY_OK to a distinct value when you hit the
non-6023 branch inside the buy loop (the branch that calls record "PumpSwap"
"buy --force-amm --confirm" "FAIL"), then update the final conditional that
currently checks [[ "$AMM_BUY_OK" == "false" ]] && [[ -z "$AMM_MINT" ]] to also
ensure no non-overflow failure marker is set before recording the 6023 ISSUE;
this ensures the existing record call for non-6023 failures remains
authoritative and prevents misreporting 6023.
In `@src/pumpfun_cli/protocol/instructions.py`:
- Around line 366-369: The instruction definition for extend_account incorrectly
marks the user account as a signer; update the AccountMeta for variable user in
the accounts list inside extend_account to is_signer=False (it is currently
True) to match the IDL, and also flip the corresponding assertion in
tests/test_protocol/test_instructions.py from True to False so the unit test
expects user not to be a signer; locate the accounts list where
AccountMeta(pubkey=user, ...) is created and the matching test assertion to
change both in tandem.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1cd7f6d8-7048-4cca-b896-8520b7663c7b
📒 Files selected for processing (6)
scripts/mainnet-test.shsrc/pumpfun_cli/protocol/instructions.pytests/test_protocol/test_extras_instructions.pytests/test_protocol/test_instructions.pytests/test_protocol/test_pumpswap_instructions.pytests/test_protocol/test_token_types.py
- extend_account: fix user account is_signer=True → False per IDL; update matching test assertion to expect is_signer=False - mainnet-test.sh: introduce AMM_OVERFLOW_ONLY flag so non-6023 AMM failures are not misclassified as "All graduated pools hit error 6023" - mainnet-test.sh: validate parsed mint from --json launch output before recording PASS; fall back to FAIL with "missing mint" message for both launch and launch --buy branches Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
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 `@scripts/mainnet-test.sh`:
- Line 374: The command capturing LAUNCH_OUT intentionally leaves $LAUNCH_IMG
unquoted which triggers ShellCheck SC2086 and is fragile for paths with spaces;
update the invocation of `uv run pumpfun ... $LAUNCH_IMG` to build the argument
list safely (e.g., construct an array or conditionally append the `--image`
argument) so that when LAUNCH_IMG is non-empty it is passed as a single argument
and when empty nothing is added; adjust the code that sets or uses LAUNCH_OUT
(the `uv run pumpfun` call) and apply the same change to the other occurrence
noted around line 394.
- Around line 370-372: The script currently creates /tmp/e2e_test_token.png
using a Python PIL one-liner and silences failures (the python3 -c "from PIL
import Image; ..." call), causing LAUNCH_IMG to remain empty with no
explanation; update the script to either document the PIL dependency in the
script header and usage comments and/or add a runtime guard: attempt to import
PIL (or run the existing python3 -c import) and if it fails echo a clear warning
like "PIL (Pillow) not installed; continuing without launch image" before
leaving LAUNCH_IMG empty, otherwise create the image and set LAUNCH_IMG="--image
/tmp/e2e_test_token.png" — modify the python3 invocation and the LAUNCH_IMG
logic in the snippet that sets LAUNCH_IMG to implement this check and message.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 942cc52c-9cbf-4100-80c0-48dcbad8fa4e
📒 Files selected for processing (3)
scripts/mainnet-test.shsrc/pumpfun_cli/protocol/instructions.pytests/test_protocol/test_instructions.py
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
1 participant
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
Comprehensive audit of all instruction builders against the pump.fun and PumpSwap AMM IDL files revealed 8 discrepancies between our instruction construction and the on-chain program expectations. These bugs caused PumpSwap AMM buys to fail with error 6023 (Overflow) due to corrupted instruction data, and introduced incorrect account flags that could cause subtle issues.
Changes
_TRACK_VOLUMEencoding: Fixed frombytes([1, 1])(2 bytes) tobytes([1])(1 byte). The IDL definesOptionBoolasstruct { bool }— a single byte, not BorshOption<bool>(2 bytes). The extra byte corrupted on-chain instruction parsing._TRACK_VOLUMEentirely — mainnet transactions confirm 24-byte data (8 disc + 8 + 8, no track_volume)._TRACK_VOLUME(already confirmed by mainnet)._TRACK_VOLUME(1 byte) — mainnet shows 25-byte data.create_v2: ChangedMAYHEM_PROGRAM_IDfromis_writable=Falsetois_writable=Trueper IDL.extend_account: Changeduserfromis_writable=Truetois_writable=Falseper IDL.claim_cashback: Changeduserfromis_signer=Truetois_signer=Falseper IDL.collect_coin_creator_fee: Added missing 8th account (PUMP_AMM_PROGRAM) per IDL.Layers Touched
protocol/instructions.py— All instruction builder fixesscripts/mainnet-test.sh— PumpSwap pool fallback for e2e teststests/test_protocol/— 8 new tests + updated assertionsDoes this affect transaction construction or signing?
Yes — instruction data encoding and account flags are corrected to match IDL specifications. PumpSwap buy transactions now send 25 bytes (was 26), bonding curve buy sends 24 bytes (was 26), and several account writable/signer flags are corrected.
Test Plan
🤖 Generated with Claude Code
Protocol-Level Transaction Construction Changes
This PR modifies instruction builders in src/pumpfun_cli/protocol/instructions.py that are directly used to construct and send real Solana transactions (these builders are used by transaction paths invoked from client code — e.g., trade, pumpswap, launch flows — and exercised by the mainnet e2e script). Any change here affects on-chain funds flow and wallet/authorization behavior.
Instruction Data Encoding Changes
Account Metadata (Signer/Writable Flags) & Account List Changes
Aligned account metas with the IDL:
Implication: transaction construction, required signatures, and which accounts may be mutated by on-chain programs changed; callers must build transactions with the updated signer/writable layout.
Code Paths that Send Transactions
E2E Script & Operational Behavior
Security & Wallet Implications
Testing & Validation
Architecture & Layer Violations
Summary: This PR corrects critical instruction encoding and account metadata to match the IDL, fixes causes of on-chain parsing/overflow failures (6023) for PumpSwap buys, and updates end-to-end test behavior. Because these builders are used to construct real-value transactions, the signer/writable changes (especially removal of signer requirements) should be reviewed for intended authorization semantics against the on-chain program IDL and security expectations before release.