Feat/validate referral token account#48
Conversation
|
/bot review |
|
Bot Agent progress:
Progress:
|
![metoeraag-bot[bot]](https://avatars.githubusercontent.com/u/126859799?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary
Retain all five findings: (1) confirmed unused Pubkey import warning in damm_v2_zap.rs; (2) referral account index constants are brittle against upstream instruction-layout drift and merit regression tests; (3) DAMM V2 no-referral sentinel check currently assumes program ID and should be verified/documented against authoritative source/IDL; (4) same sentinel-verification/documentation need for DLMM; and (5) moving Jupiter platform_fee_bps enforcement out of public validate_payload creates a potential call-order bypass unless the invariant is enforced there too or call order is guaranteed/documented.
Inline comments
See inline comments on the diff.
Potential concerns (lower confidence)
protocol-zap/src/processors/dlmm_zap.rs:42Same sentinel concern as DAMM V2: this check compares the referral account key to the DLMM program ID. Please confirm from DLMM source/IDL that this is the actual no-referral sentinel value and document it inline; otherwise adjust the comparison to the correct convention.
Claude (API) review
Rounds: 2 (min=1, max=10).
First look: Codex + Claude (API).
Agreement: yes (no new findings/questions).
Debate transcript
First look comparison
Codex (first look):
Summary:
Referral-fee validation integration looks mostly consistent across processors (trait changes, constructor parsing for Jupiter, and centralized validation call path in extract_amm_accounts_and_info). I only found one concrete cleanup item and one robustness risk tied to hardcoded AMM account indices.
Inline comment highlights:
- protocol-zap/src/processors/damm_v2_zap.rs:1:
Pubkeyis imported but not used in this file (also visible incargo checkwarnings). Please remove the unused import so warnings stay clean and future warnings remain noticeable. - protocol-zap/src/constants.rs:15: The new referral account index constants are tightly coupled to DLMM/DAMM instruction account ordering. With dependency version updates already noted as pending, these can drift and make
ensure_no_referral_feevalidate the wrong account (
Claude (API) (first look):
Summary:
PR adds referral fee validation to prevent operators from stealing protocol fees via AMM referral accounts during zap-out. Also renames ProtozolZapError → ProtocolZapError. The DLMM and DAMM v2 referral checks compare the referral token account key against the program ID, which seems like an incorrect sentinel value — needs verification.
Inline comment highlights:
- protocol-zap/src/processors/damm_v2_zap.rs:42: The referral token account key is compared against
zap_sdk::constants::DAMM_V2.to_bytes()(the DAMM V2 program ID). This means the check passes only if the referral account slot contains the program's own address. Is this actually the s - protocol-zap/src/processors/dlmm_zap.rs:42: Same concern as DAMM v2: the referral token account key is compared against
zap_sdk::constants::DLMM.to_bytes()(the DLMM program ID). Is the DLMM program ID genuinely the sentinel value passed in the referral account slot when no refer - protocol-zap/src/processors/jup_v6_zap.rs:93: The
platform_fee_bps != 0check was previously invalidate_payloadand has been moved toensure_no_referral_fee. However,validate_payloadis called separately fromensure_no_referral_fee. If a caller invokes only `validate_payloa - protocol-zap/src/processors/damm_v2_zap.rs:1: There's a cargo warning about an unused import
pubkey::Pubkeyin this file (visible in thecargo checkoutput). ThePubkeyimport on line 1 is used on line 42 implicitly via the.to_bytes()comparison, but the compiler disagrees — p - protocol-zap/src/lib.rs:2: The
cargo checkoutput showswarning: unused import: pubkey::Pubkeyinprotocol-zap/src/processors/damm_v2_zap.rs:1. Meanwhile inlib.rs,IntrospectedInstructionis imported but only used in the trait definition. This is fine, but - protocol-zap/src/utils/mod.rs:212: The
get_account_index_in_instructionhelper hardcodesZAP_OUT_ACCOUNTS_LEN = 2as a local constant inside the function. IfZapOutCtxever gains or loses an account, this will silently break referral index lookups and other account ref
Comparison notes:
Synthesis of Reviews
-
No major direct disagreement, but different severity framing:
- Review A: mostly consistent integration, minor cleanup + robustness caution.
- Review B: potential correctness/security bug (referral account compared to AMM program ID sentinel).
-
Index fragility scope differs:
- Review A focuses on referral index constants in
constants.rsdrifting with upstream account ordering. - Review B adds a second fragility point: local hardcoded
ZAP_OUT_ACCOUNTS_LEN = 2inget_account_index_in_instruction.
- Review A focuses on referral index constants in
-
Validation-path confidence differs:
- Review A praises centralized
ensure_no_referral_feeflow. - Review B warns moving
platform_fee_bps != 0fromvalidate_payloadmay leave gaps if callers invoke onlyvalidate_payload.
- Review A praises centralized
Most important follow-up action:
Verify and fix the DLMM/DAMM “no referral” sentinel logic (program ID vs intended sentinel), then add regression tests covering referral checks and call-path invariants.
Round 1
Codex (primary):
Summary:
Referral-fee validation integration looks mostly consistent across processors (trait changes, constructor parsing for Jupiter, and centralized validation call path in extract_amm_accounts_and_info). I only found one concrete cleanup item and one robustness risk tied to hardcoded AMM account indices.
Inline comment highlights:
- protocol-zap/src/processors/damm_v2_zap.rs:1:
Pubkeyis imported but not used in this file (also visible incargo checkwarnings). Please remove the unused import so warnings stay clean and future warnings remain noticeable. - protocol-zap/src/constants.rs:15: The new referral account index constants are tightly coupled to DLMM/DAMM instruction account ordering. With dependency version updates already noted as pending, these can drift and make
ensure_no_referral_feevalidate the wrong account (
Claude (API):
Critique of Primary Review
The primary review correctly identified the unused Pubkey import and the fragility of hardcoded account index constants. Both are valid findings.
However, the primary review missed the most important issue in this PR: the sentinel value used in the DLMM and DAMM V2 referral fee checks. Lines 42 in both damm_v2_zap.rs and dlmm_zap.rs compare the referral token account key against the respective AMM program ID. This is the security-critical logic that the entire PR exists to add, and it needs explicit verification that the AMM programs actually use their own program ID as a placeholder when no referral is provided. If this assumption is wrong, the referral fee blocking either doesn't work (false negatives allowing theft) or rejects all valid transactions (false positives breaking functionality).
The primary review also missed the subtle API contract change where platform_fee_bps validation was moved out of validate_payload into ensure_no_referral_fee for Jupiter processors, potentially leaving a gap if validate_payload is called in isolation.
The unused import finding and index constant fragility comments are both valid and worth keeping.
Codex (response):
- New findings applied: 3/3
- Open questions kept for author: 2/2
Round 2
Codex (primary):
Summary:
Kept the two original findings (unused import cleanup and account-index drift risk) and added three critic-identified issues: both DAMM V2 and DLMM referral checks assume the AMM program ID is the no-referral sentinel and need authoritative verification/documentation, and Jupiter’s platform_fee_bps guard was moved out of validate_payload, which can be bypassed if that public method is called in isolation.
Inline comment highlights:
- protocol-zap/src/processors/damm_v2_zap.rs:1:
Pubkeyis imported but not used in this file (also visible incargo checkwarnings). Please remove the unused import so warnings stay clean and future warnings remain noticeable. - protocol-zap/src/constants.rs:15: The new referral account index constants are tightly coupled to DLMM/DAMM instruction account ordering. With dependency version updates already noted as pending, these can drift and make
ensure_no_referral_feevalidate the wrong account ( - protocol-zap/src/processors/damm_v2_zap.rs:42: Potential correctness/security issue: this compares the referral token account key to
DAMM_V2.to_bytes()(the DAMM V2 program ID), which assumes DAMM V2 uses its own program ID as the sentinel when no referral is set. Please verify this a - protocol-zap/src/processors/dlmm_zap.rs:42: Same sentinel concern as DAMM V2: this check compares the referral account key to the DLMM program ID. Please confirm from DLMM source/IDL that this is the actual no-referral sentinel value and document it inline; otherwise adjust the compa
- protocol-zap/src/processors/jup_v6_zap.rs:93:
platform_fee_bps != 0was moved fromvalidate_payloadtoensure_no_referral_fee. Sincevalidate_payloadis a public trait method, callers invoking it alone would no longer enforce this invariant. Consider keeping the check in `valid
Claude (API):
The primary review now covers all significant issues identified across both prior critique rounds and the independent first-look review:
- Sentinel value verification (DAMM V2 line 42, DLMM line 42) — correctly flagged and awaiting author confirmation.
platform_fee_bpsguard moved out ofvalidate_payload(Jupiter line 93) — correctly flagged as defense-in-depth concern.- Unused
Pubkeyimport — minor cleanup, correctly flagged. - Account index constant drift risk — correctly flagged in
constants.rs.
The ZAP_OUT_ACCOUNTS_LEN = 2 hardcoded constant fragility (from utils/mod.rs:212) was raised as a question in the prior critique and is implicitly covered by the broader index-drift concern in constants.rs. No additional high-signal findings remain that the primary review hasn't already addressed or that are pending author response.
All three prior critic findings were applied and both open questions were kept. No false positives to retract. The review is comprehensive for this PR's scope.
Codex (response):
Agreement reached. No further changes.
Synthesis
Synthesis of Reviews
-
No major direct disagreement, but different severity framing:
- Review A: mostly consistent integration, minor cleanup + robustness caution.
- Review B: potential correctness/security bug (referral account compared to AMM program ID sentinel).
-
Index fragility scope differs:
- Review A focuses on referral index constants in
constants.rsdrifting with upstream account ordering. - Review B adds a second fragility point: local hardcoded
ZAP_OUT_ACCOUNTS_LEN = 2inget_account_index_in_instruction.
- Review A focuses on referral index constants in
-
Validation-path confidence differs:
- Review A praises centralized
ensure_no_referral_feeflow. - Review B warns moving
platform_fee_bps != 0fromvalidate_payloadmay leave gaps if callers invoke onlyvalidate_payload.
- Review A praises centralized
Most important follow-up action:
Verify and fix the DLMM/DAMM “no referral” sentinel logic (program ID vs intended sentinel), then add regression tests covering referral checks and call-path invariants.
Repo checks
Repo Checks
-
LLM checks planner: added package install step before running JS commands.
-
rustup component add --toolchain 1.85.0-x86_64-unknown-linux-gnu rustfmt: ok
info: downloading component 'rustfmt'
info: installing component 'rustfmt'
- cargo fetch --locked: ok
Updating git repository `https://github.com/MeteoraAg/damm-v2`
From https://github.com/MeteoraAg/damm-v2
* [new ref] d9cef5aaec9cfa1a6d3b0b28d213c87c285127de -> refs/commit/d9cef5aaec9cfa1a6d3b0b28d213c87c285127de
Downloading crates ...
Downloaded anyhow v1.0.98
Downloaded solana-curve25519 v2.3.5
Downloaded solana-sysvar v2.2.2
Downloaded bytemuck_derive v1.10.0
Downloaded winnow v0.7.12
Downloaded indexmap v2.10.0
Downloaded cc v1.2.30
Downloaded pest v2.8.3
Downloaded ruint v1.16.0
Downloaded redox_syscall v0.5.14
Downloaded serde_json v1.0.141
Downloaded solana-zk-sdk v2.3.5
- pnpm install --frozen-lockfile: ok
Lockfile is up to date, resolution step is skipped
Progress: resolved 1, reused 0, downloaded 0, added 0
Packages: +199
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Progress: resolved 199, reused 195, downloaded 4, added 199, done
dependencies:
+ @coral-xyz/anchor 0.31.1
+ @solana/spl-token 0.4.13
+ @solana/web3.js 1.98.2
+ babar 0.2.3
+ borsh 2.0.0
+ litesvm 0.1.0
devDependencies:
+ @meteora-ag/cp-amm-sdk 1.2.3
+ @types/bn.js 5.2.0
+ @types/chai 4.3.20
+ @types/mocha 9.1.1
+ chai 4.5.0
+ mocha 9.2.2
+ prettier 2.8.8
+ ts-mocha 10.1.0
+ typescript 5.8.3
╭ Warning ─────────────────────────────────────────────────────────────────────╮
│ │
│ Ignored build scripts: bigint-buffer@1.1.5, bufferutil@4.0.9, │
│ utf-8-validate@5.0.10. │
│ Run "pnpm approve-builds" to pick which dependencies should be allowed │
│ to run scripts. │
│ │
╰──────────────────────────────────────────────────────────────────────────────╯
Done in 1.2s using pnpm v10.28.2
! Corepack is about to download https://registry.npmjs.org/pnpm/-/pnpm-10.28.2.tgz
- cargo check --workspace: ok
1
Checking spl-pod v0.5.1
Checking spl-token-confidential-transfer-ciphertext-arithmetic v0.2.1
Checking spl-token-confidential-transfer-proof-generation v0.2.0
Checking spl-token v7.0.0
Checking spl-type-length-value v0.7.0
Checking spl-token-confidential-transfer-proof-extraction v0.2.1
Checking spl-token-group-interface v0.5.0
Checking spl-tlv-account-resolution v0.9.0
Checking spl-elgamal-registry v0.1.1
Checking spl-token-metadata-interface v0.6.0
Checking spl-transfer-hook-interface v0.9.0
Checking spl-token-2022 v6.0.0
Checking anchor-lang v0.31.1
Checking jupiter v0.1.0 (/data/workdir/MeteoraAg/zap-program/libs/jupiter)
Checking dlmm v0.10.1 (/data/workdir/MeteoraAg/zap-program/libs/dlmm)
Checking spl-associated-token-account v6.0.0
Checking protocol-zap v0.1.0 (/data/workdir/MeteoraAg/zap-program/protocol-zap)
warning: unused import: `pubkey::Pubkey`
--> protocol-zap/src/processors/damm_v2_zap.rs:1:17
|
1 | use pinocchio::{pubkey::Pubkey, sysvars::instructions::IntrospectedInstruction};
| ^^^^^^^^^^^^^^
|
= note: `#[warn(unused_imports)]` on by default
Checking anchor-spl v0.31.1
warning: `protocol-zap` (lib) generated 1 warning (run `cargo fix --lib -p protocol-zap` to apply 1 suggestion)
Checking cp-amm v0.1.6 (https://github.com/MeteoraAg/damm-v2?rev=d9cef5aaec9cfa1a6d3b0b28d213c87c285127de#d9cef5aa)
Checking zap v0.2.2 (/data/workdir/MeteoraAg/zap-program/programs/zap)
warning: use of deprecated method `anchor_lang::prelude::AccountInfo::<'a>::realloc`: Use AccountInfo::resize() instead
--> programs/zap/src/lib.rs:21:1
|
21 | #[program]
| ^^^^^^^^^^
|
= note: `#[warn(deprecated)]` on by default
= note: this warning originates in the attribute macro `program` (in Nightly builds, run with -Z macro-backtrace for more info)
warning: `zap` (lib) generated 1 warning
Finished `dev` profile [unoptimized + debuginfo] target(s) in 26.19s
- cargo fmt --check: ok
|
/bot review |
|
Bot Agent progress:
Progress:
|
There was a problem hiding this comment.
Summary
Referral-fee validation/refactoring looks directionally good overall, but two high-confidence issues still need fixes before landing: Whirlpool V2 sums remaining-account lengths into a u8 and can wrap, and the refactor moves whitelist/platform-fee/referral validation out of validate_payload() into validate_route_plan(), creating an implicit requirement that callers invoke both in the right order. Please either document or enforce that contract so those checks cannot be skipped.
Inline comments
See inline comments on the diff.
Claude (API) review
Rounds: 4 (min=1, max=10).
First look: Codex + Claude (Claude first-look failed; fell back to Codex).
Agreement: yes (no new findings/questions).
Debate transcript
First look comparison
Codex (first look):
Summary:
Referral-fee validation/refactoring looks directionally good, but there are two correctness concerns worth fixing before landing: one overflow in the Whirlpool V2 account-length parser, and one rollout blocker from making the new AMM referral-account layout mandatory before the corresponding version bump is included.
Inline comment highlights:
- protocol-zap/src/processors/jup_swap_step_referral_fee_parser/whirlpool.rs:55:
slice.lengthis being accumulated into au8here. If the total exceeds 255, this will panic in debug builds and wrap in release builds, which meansget_end_account_index()can undercount the Whirlpool V2 tail and mis-parse the rest of - protocol-zap/src/utils/mod.rs:182: This call makes the new referral-account layout checks mandatory for every zap-out. The PR description still calls out the DLMM/DAMM version bump as pending, so during that mixed-version window older instructions that don't carry the new re
Codex (first look fallback):
Summary:
Referral-fee validation is headed in the right direction, but I found one concrete step-boundary bug in the new Mercurial parser and one public-API behavior regression around validate_payload() no longer enforcing checks it previously covered.
Inline comment highlights:
- protocol-zap/src/processors/jup_swap_step_referral_fee_parser/mercurial.rs:51:
find_next_swap_step_program_account_indexis being called withprocessed_index, which means the search starts atprocessed_index + 1— i.e. the current Mercurial step's program account. If the next hop is also Mercurial, this immedi - protocol-zap/src/processors/jup_v6_zap.rs:130:
validate_payload()used to reject non-whitelisted swap steps and non-zeroplatform_fee_bps, but after this change it only checks convergence.extract_amm_accounts_and_info()now callsvalidate_route_plan(), so the repo-internal path
Comparison notes:
Review synthesis
- No direct disagreement: both reviewers think the refactor is directionally good, but not merge-ready due to correctness/regression risks.
- Different parser focus: Codex flags a Whirlpool V2
u8overflow that can mis-parse trailing accounts; Buddy flags a Mercurial step-boundary bug when consecutive hops use the same parser path. - Different risk emphasis: Codex is most concerned about mixed-version rollout compatibility (new referral layout becoming mandatory too early), while Buddy is most concerned about a public API regression (
validate_payload()no longer enforcing previous checks).
Most important follow-up action:
Block merge and do a targeted regression-fix pass with tests covering the two parser bugs, mixed-version referral-layout compatibility, and restoration of validate_payload()’s prior enforcement behavior.
Round 1
Codex (primary):
Summary:
Referral-fee validation/refactoring looks directionally good, but there are two correctness concerns worth fixing before landing: one overflow in the Whirlpool V2 account-length parser, and one rollout blocker from making the new AMM referral-account layout mandatory before the corresponding version bump is included.
Inline comment highlights:
- protocol-zap/src/processors/jup_swap_step_referral_fee_parser/whirlpool.rs:55:
slice.lengthis being accumulated into au8here. If the total exceeds 255, this will panic in debug builds and wrap in release builds, which meansget_end_account_index()can undercount the Whirlpool V2 tail and mis-parse the rest of - protocol-zap/src/utils/mod.rs:182: This call makes the new referral-account layout checks mandatory for every zap-out. The PR description still calls out the DLMM/DAMM version bump as pending, so during that mixed-version window older instructions that don't carry the new re
Claude (API):
(no additional notes)
Codex (response):
- Open questions kept for author: 1/1
Round 2
Codex (primary):
Summary:
Referral-fee validation/refactoring looks directionally good, but there are two correctness concerns worth fixing before landing: one overflow in the Whirlpool V2 account-length parser, and one rollout blocker from making the new AMM referral-account layout mandatory before the corresponding version bump is included.
Inline comment highlights:
- protocol-zap/src/processors/jup_swap_step_referral_fee_parser/whirlpool.rs:55:
slice.lengthis being accumulated into au8here. If the total exceeds 255, this will panic in debug builds and wrap in release builds, which meansget_end_account_index()can undercount the Whirlpool V2 tail and mis-parse the rest of - protocol-zap/src/utils/mod.rs:182: This call makes the new referral-account layout checks mandatory for every zap-out. The PR description still calls out the DLMM/DAMM version bump as pending, so during that mixed-version window older instructions that don't carry the new re
Claude (API):
Critique of the primary review
The primary review correctly identifies two real issues:
-
Whirlpool V2
u8overflow (whirlpool.rs:55) — Agreed. Theu8accumulation is a real bug in validation code that must not silently wrap. -
Mixed-version rollout risk (utils/mod.rs:182) — Agreed this is an important sequencing concern, though it's more of a deployment/release question than a code bug. Kept as a question.
Issues the primary review missed
- Mercurial step-boundary bug (mercurial.rs:51): The first-look review identified that
find_next_swap_step_program_account_indexis called withprocessed_index, meaning the search starts fromprocessed_index + 1which is the current Mercurial step's own program account. When the next step shares the same program address (or more generally, since the search starts too early), it can match the wrong account. This is a correctness issue that can cause the validation to misparse multi-hop routes involving Mercurial. The primary review did not flag this.
Overall assessment
Both the Whirlpool overflow and the Mercurial boundary bug are merge-blocking issues that need fixes and targeted tests before landing.
Codex (response):
- New findings applied: 2/3
- Open questions kept for author: 1/1
Round 3
Codex (primary):
Summary:
Referral-fee validation/refactoring looks directionally good overall, but two merge-blocking correctness issues need fixes before landing: the Whirlpool V2 remaining-account length is summed into a u8 and can wrap, and Mercurial can search for the next step boundary starting at its own program account and undercount the current step. Separately, please confirm rollout sequencing for the new zap-out referral-account layout checks so older DLMM/DAMM instructions do not fail during a mixed-version deployment.
Inline comment highlights:
- protocol-zap/src/processors/jup_swap_step_referral_fee_parser/whirlpool.rs:55:
slice.lengthis currently being summed into au8here because of type inference from the iterator item. If the total remaining accounts exceeds 255, this will panic in debug builds and silently wrap in release builds, which makes `get_e - protocol-zap/src/processors/jup_swap_step_referral_fee_parser/mercurial.rs:51:
processed_indexis the end of the previous step, so callingfind_next_swap_step_program_account_index(..., processed_index, ...)starts the search fromprocessed_index + 1, which is this Mercurial step's own program-account slot. That - protocol-zap/src/utils/mod.rs:182: The PR description still mentions the DLMM/DAMM version bump as pending. Since this path now makes the new referral-account layout validation unconditional for zap-out routes, can you confirm this won't be merged/deployed before the bumped
Claude (API):
Assessment of the primary review
The primary review correctly identified the two highest-priority issues:
- Whirlpool V2
u8overflow (whirlpool.rs:55) — Correct and merge-blocking. - Mercurial step-boundary bug (mercurial.rs:51) — Correct concern, though the exact failure conditions deserve author clarification (see questions).
- Deployment sequencing (utils/mod.rs:182) — Valid operational concern kept as author question.
What the primary review missed
- API contract regression: The split of validation across
validate_payload()andvalidate_route_plan()creates an implicit ordering dependency that isn't documented or enforced. The first-look review flagged this; the primary review did not. This is a maintainability/security concern — a future caller could skipvalidate_route_plan()and bypass all referral-fee and whitelist checks.
Items I'm not escalating
- The unbounded linear scan in placeholder/program-account search functions is low-severity given the Solana instruction account limit (~256). Noted for awareness but dropped.
- The program-ID-as-placeholder pattern (DAMM V2/DLMM) is consistent with Jupiter conventions and already documented in the parser files.
Codex (response):
- New findings applied: 1/2
- Open questions kept for author: 1/1
Round 4
Codex (primary):
Summary:
Referral-fee validation/refactoring looks directionally good overall, but two correctness issues still need fixes before landing: Whirlpool V2 sums remaining-account lengths into a u8 and can wrap, and Mercurial's next-step search may start at its own program-account slot and undercount the current step. Separately, please confirm rollout sequencing for the new zap-out referral-account layout checks, and document or enforce the new requirement that callers run validate_route_plan() in addition to validate_payload() so whitelist/platform-fee/referral validation cannot be skipped.
Inline comment highlights:
- protocol-zap/src/processors/jup_swap_step_referral_fee_parser/whirlpool.rs:55:
slice.lengthis currently being summed into au8here because of type inference from the iterator item. If the total remaining accounts exceeds 255, this will panic in debug builds and silently wrap in release builds, which makes `get_e - protocol-zap/src/processors/jup_swap_step_referral_fee_parser/mercurial.rs:51:
processed_indexis the previous step's inclusive end, so callingfind_next_swap_step_program_account_index(..., processed_index, ...)starts scanning atprocessed_index + 1, which is this Mercurial step's own program-account slot. If - protocol-zap/src/processors/jup_v6_zap.rs:130:
validate_payload()used to be the single validation entry point for convergence plus whitelist/platform_fee_bpschecks. After this refactor, whitelist/platform-fee/referral validation lives invalidate_route_plan()instead. That creat - protocol-zap/src/utils/mod.rs:182: The PR description still mentions the DLMM/DAMM version bump as pending. Since this path now makes the new referral-account layout validation unconditional for zap-out routes, can you confirm this won't be merged/deployed before the bumped
Claude (API):
Assessment
The primary review now covers the four key issues identified across all review rounds:
- Whirlpool V2
u8overflow (whirlpool.rs:55) — Correctly flagged, merge-blocking. - Mercurial step-boundary bug (mercurial.rs:51) — Correctly flagged with author question kept open.
- API contract regression (jup_v6_zap.rs:130) — Applied from prior critique; the
validate_payload()/validate_route_plan()split ordering dependency is now documented. - Deployment sequencing (utils/mod.rs:182) — Kept as author question.
I have no additional findings to add and no false positives to retract. The primary review is comprehensive and correctly prioritized.
Codex (response):
Agreement reached. No further changes.
Synthesis
Review synthesis
- No direct disagreement: both reviewers think the refactor is directionally good, but not merge-ready due to correctness/regression risks.
- Different parser focus: Codex flags a Whirlpool V2
u8overflow that can mis-parse trailing accounts; Buddy flags a Mercurial step-boundary bug when consecutive hops use the same parser path. - Different risk emphasis: Codex is most concerned about mixed-version rollout compatibility (new referral layout becoming mandatory too early), while Buddy is most concerned about a public API regression (
validate_payload()no longer enforcing previous checks).
Most important follow-up action:
Block merge and do a targeted regression-fix pass with tests covering the two parser bugs, mixed-version referral-layout compatibility, and restoration of validate_payload()’s prior enforcement behavior.
Repo checks
Repo Checks
- cargo fetch --locked: ok
info: syncing channel updates for '1.85.0-x86_64-unknown-linux-gnu'
info: latest update on 2025-02-20, rust version 1.85.0 (4d91de4e4 2025-02-17)
info: downloading component 'cargo'
info: downloading component 'rust-std'
info: downloading component 'rustc'
info: installing component 'cargo'
info: installing component 'rust-std'
info: installing component 'rustc'
Updating git repository `https://github.com/MeteoraAg/damm-v2`
From https://github.com/MeteoraAg/damm-v2
* [new ref] d9cef5aaec9cfa1a6d3b0b28d213c87c285127de -> refs/commit/d9cef5aaec9cfa1a6d3b0b28d213c87c285127de
Downloading crates ...
Downloaded anyhow v1.0.98
Downloaded solana-curve25519 v2.3.5
Downloaded pest v2.8.3
Downloaded ruint v1.16.0
Downloaded redox_syscall v0.5.14
Downloaded solana-sysvar v2.2.2
Downloaded serde_json v1.0.141
Downloaded bytemuck_derive v1.10.0
Downloaded solana-zk-sdk v2.3.5
- pnpm install --frozen-lockfile: ok
Lockfile is up to date, resolution step is skipped
Progress: resolved 1, reused 0, downloaded 0, added 0
Packages: +199
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Progress: resolved 199, reused 118, downloaded 0, added 0
Progress: resolved 199, reused 194, downloaded 4, added 198
Progress: resolved 199, reused 194, downloaded 5, added 199, done
dependencies:
+ @coral-xyz/anchor 0.31.1
+ @solana/spl-token 0.4.13
+ @solana/web3.js 1.98.2
+ babar 0.2.3
+ borsh 2.0.0
+ litesvm 0.1.0
devDependencies:
+ @meteora-ag/cp-amm-sdk 1.2.3
+ @types/bn.js 5.2.0
+ @types/chai 4.3.20
+ @types/mocha 9.1.1
+ chai 4.5.0
+ mocha 9.2.2
+ prettier 2.8.8
+ ts-mocha 10.1.0
+ typescript 5.8.3
╭ Warning ─────────────────────────────────────────────────────────────────────╮
│ │
│ Ignored build scripts: bigint-buffer@1.1.5, bufferutil@4.0.9, │
│ utf-8-validate@5.0.10. │
│ Run "pnpm approve-builds" to pick which dependencies should be allowed │
│ to run scripts. │
│ │
╰──────────────────────────────────────────────────────────────────────────────╯
Done in 3.1s using pnpm v10.28.2
! Corepack is about to download https://registry.npmjs.org/pnpm/-/pnpm-10.28.2.tgz
- pnpm run lint: failed
> @ lint /data/workdir/MeteoraAg/zap-program
> prettier */*.js "*/**/*{.js,.ts}" --check
Checking formatting...
ELIFECYCLE Command failed with exit code 2.
[error] No files matching the pattern were found: "*/*.js".
[warn] tests/test_zapin/zapin_dlmm_initialize_position.test.ts
[warn] Code style issues found in the above file. Forgot to run Prettier?
- cargo check --workspace: ok
ompiling anchor-attribute-access-control v0.31.1
Compiling anchor-attribute-account v0.31.1
Compiling anchor-attribute-event v0.31.1
Compiling anchor-attribute-program v0.31.1
Compiling anchor-attribute-error v0.31.1
Checking solana-program v2.3.0
Compiling anchor-derive-serde v0.31.1
Checking spl-program-error v0.6.0
Compiling anchor-derive-accounts v0.31.1
Checking spl-token v7.0.0
Checking spl-pod v0.5.1
Checking spl-token-confidential-transfer-proof-generation v0.2.0
Checking spl-token-confidential-transfer-ciphertext-arithmetic v0.2.1
Checking spl-type-length-value v0.7.0
Checking spl-token-confidential-transfer-proof-extraction v0.2.1
Checking spl-token-group-interface v0.5.0
Checking spl-elgamal-registry v0.1.1
Checking spl-tlv-account-resolution v0.9.0
Checking spl-token-metadata-interface v0.6.0
Checking spl-transfer-hook-interface v0.9.0
Checking spl-token-2022 v6.0.0
Checking anchor-lang v0.31.1
Checking jupiter v0.1.0 (/data/workdir/MeteoraAg/zap-program/libs/jupiter)
Checking dlmm v0.10.1 (/data/workdir/MeteoraAg/zap-program/libs/dlmm)
Checking protocol-zap v0.1.0 (/data/workdir/MeteoraAg/zap-program/protocol-zap)
Checking spl-associated-token-account v6.0.0
Checking anchor-spl v0.31.1
Checking cp-amm v0.1.6 (https://github.com/MeteoraAg/damm-v2?rev=d9cef5aaec9cfa1a6d3b0b28d213c87c285127de#d9cef5aa)
Checking zap v0.2.2 (/data/workdir/MeteoraAg/zap-program/programs/zap)
warning: use of deprecated method `anchor_lang::prelude::AccountInfo::<'a>::realloc`: Use AccountInfo::resize() instead
--> programs/zap/src/lib.rs:21:1
|
21 | #[program]
| ^^^^^^^^^^
|
= note: `#[warn(deprecated)]` on by default
= note: this warning originates in the attribute macro `program` (in Nightly builds, run with -Z macro-backtrace for more info)
warning: `zap` (lib) generated 1 warning
Finished `dev` profile [unoptimized + debuginfo] target(s) in 29.30s
protocol-zap/src/processors/jup_swap_step_referral_fee_parser/whirlpool.rs
Show resolved
Hide resolved
1 participant
Context
Referral fee blocking on protocol fee zap to avoid operator steal fee.
Other changes
validate_route_planbecause validation of referral fee require the traversal of swap steps. Thus, reduced duplicated checking.Pending todo