Open
Conversation
- Support Zap-In to damm-V2 Compounding Pool Fee Mode - Support Zap-In to single-sided damm-v2 pool
Collaborator
Author
|
/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.
Verdict
Request changes
Summary
Posted 1 validated inline finding(s) after final filtering.
Findings
- [P1]
package.json:41These dependency additions/version bumps also need a correspondingbun.lockupdate. In the current state,bun install --frozen-lockfilefails because Bun wants to rewrite the lockfile, which prevents a clean install and is the direct cause of the downstreamtypecheck/buildfailures in this run. Please regenerate and commit the Bun lockfile (or remove Bun from the required workflow if it's no longer supported).
Repo checks
Repo Checks
- bun install --frozen-lockfile: failed
bun install v1.3.2 (b131639c)
Resolving dependencies
Resolved, downloaded and extracted [309]
error: lockfile had changes, but lockfile is frozen
note: try re-running without --frozen-lockfile and commit the updated lockfile
- bun run typecheck: failed
error TS2688: Cannot find type definition file for 'chai'.
The file is in the program because:
Entry point of type library 'chai' specified in compilerOptions
error TS2688: Cannot find type definition file for 'mocha'.
The file is in the program because:
Entry point of type library 'mocha' specified in compilerOptions
$ tsc
- bun run build: failed
$ rm -rf dist && tsup src/index.ts --format esm,cjs --dts
/usr/bin/bash: line 1: tsup: command not found
error: script "build" exited with code 127
Collaborator
Author
|
/bot review |
|
Bot Agent progress:
Progress:
|
There was a problem hiding this comment.
Verdict
Request changes
Summary
Posted 1 validated inline finding(s) after final filtering.
Findings
- [P1]
package.json:5The published entrypoints don’t match the actual tsup outputs. Repo-local build logs show ESM atdist/index.mjsand CJS atdist/index.js, butmain/exports.requirestill point todist/index.cjs, andmodule/exports.importpoint todist/index.js(the CJS artifact). That meansrequire('@meteora-ag/zap-sdk')will resolve to a missing file, and ESM consumers will be pointed at the wrong bundle format. Please either change the tsup output extensions to emit the declared filenames, or updatemain/module/exportsto match the generated files before publishing 1.3.0.
Repo checks
Repo Checks
- pnpm install --frozen-lockfile: ok
Lockfile is up to date, resolution step is skipped
Progress: resolved 1, reused 0, downloaded 0, added 0
Packages: +371
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Progress: resolved 371, reused 87, downloaded 0, added 0
Progress: resolved 371, reused 355, downloaded 12, added 28
Progress: resolved 371, reused 355, downloaded 16, added 371, done
dependencies:
+ @coral-xyz/anchor 0.31.1
+ @meteora-ag/cp-amm-sdk 1.4.1
+ @meteora-ag/dlmm 1.9.0
+ @solana/spl-token 0.4.13
+ @solana/web3.js 1.98.4
+ bn.js 5.2.2
+ decimal.js 10.6.0
+ invariant 2.2.4
+ typescript 5.9.2
devDependencies:
+ @types/bn.js 5.2.0
+ @types/chai 4.3.20
+ @types/invariant 2.2.37
+ @types/mocha 9.1.1
+ bip39 3.1.0
+ chai 4.5.0
+ ed25519-hd-key 1.3.0
+ litesvm 0.6.0
+ mocha 9.2.2
+ prettier 3.8.2
+ ts-mocha 10.1.0
+ tsup 8.5.1
+ tsx 4.20.3
╭ Warning ─────────────────────────────────────────────────────────────────────╮
│ │
│ Ignored build scripts: bigint-buffer@1.1.5, bufferutil@4.0.9, │
│ esbuild@0.25.8, esbuild@0.27.7, 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.33.0
! Corepack is about to download https://registry.npmjs.org/pnpm/-/pnpm-10.33.0.tgz
- pnpm run typecheck: ok
> @meteora-ag/zap-sdk@1.3.0 typecheck /data/workdir/2962945/MeteoraAg/zap-sdk
> tsc
- pnpm run build: ok
> @meteora-ag/zap-sdk@1.3.0 build /data/workdir/2962945/MeteoraAg/zap-sdk
> rm -rf dist && tsup src/index.ts --format esm,cjs --dts
�[34mCLI�[39m Building entry: src/index.ts
�[34mCLI�[39m Using tsconfig: tsconfig.json
�[34mCLI�[39m tsup v8.5.1
�[34mCLI�[39m Target: es6
�[34mESM�[39m Build start
�[34mCJS�[39m Build start
�[32mESM�[39m �[1mdist/index.mjs �[22m�[32m157.41 KB�[39m
�[32mESM�[39m ⚡️ Build success in 63ms
�[32mCJS�[39m �[1mdist/index.js �[22m�[32m165.37 KB�[39m
�[32mCJS�[39m ⚡️ Build success in 64ms
�[34mDTS�[39m Build start
�[32mDTS�[39m ⚡️ Build success in 3247ms
�[32mDTS�[39m �[1mdist/index.d.mts �[22m�[32m114.60 KB�[39m
�[32mDTS�[39m �[1mdist/index.d.ts �[22m�[32m114.60 KB�[39m
Collaborator
Author
|
/bot review |
|
Bot Agent progress:
Progress:
|
There was a problem hiding this comment.
Verdict
Request changes
Summary
Posted 1 validated inline finding(s) after final filtering.
Findings
- [P1]
package.json:34The build now emitsdist/index.mjsfor ESM anddist/index.jsfor CJS, butexports["."].importstill points to./dist/index.js(andmoduleabove does too). That makes ESM consumers load the CJS artifact after publish, which can break/bypass proper ESM interop and leaves the generated.mjsbundle unused. Please point the ESM entrypoints at./dist/index.mjs(or change the build output names to match the export map).
Repo checks
Repo Checks
- pnpm install --frozen-lockfile: ok
Lockfile is up to date, resolution step is skipped
Progress: resolved 1, reused 0, downloaded 0, added 0
Packages: +371
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Progress: resolved 371, reused 351, downloaded 20, added 371, done
dependencies:
+ @coral-xyz/anchor 0.31.1
+ @meteora-ag/cp-amm-sdk 1.4.1
+ @meteora-ag/dlmm 1.9.0
+ @solana/spl-token 0.4.13
+ @solana/web3.js 1.98.4
+ bn.js 5.2.2
+ decimal.js 10.6.0
+ invariant 2.2.4
+ typescript 5.9.2
devDependencies:
+ @types/bn.js 5.2.0
+ @types/chai 4.3.20
+ @types/invariant 2.2.37
+ @types/mocha 9.1.1
+ bip39 3.1.0
+ chai 4.5.0
+ ed25519-hd-key 1.3.0
+ litesvm 0.6.0
+ mocha 9.2.2
+ prettier 3.8.2
+ ts-mocha 10.1.0
+ tsup 8.5.1
+ tsx 4.20.3
╭ Warning ─────────────────────────────────────────────────────────────────────╮
│ │
│ Ignored build scripts: bigint-buffer@1.1.5, bufferutil@4.0.9, │
│ esbuild@0.25.8, esbuild@0.27.7, utf-8-validate@5.0.10. │
│ Run "pnpm approve-builds" to pick which dependencies should be allowed │
│ to run scripts. │
│ │
╰──────────────────────────────────────────────────────────────────────────────╯
Done in 1.6s using pnpm v10.33.0
- pnpm run typecheck: ok
> @meteora-ag/zap-sdk@1.3.0 typecheck /data/workdir/2962945/MeteoraAg/zap-sdk
> tsc
- pnpm run build: ok
> @meteora-ag/zap-sdk@1.3.0 build /data/workdir/2962945/MeteoraAg/zap-sdk
> rm -rf dist && tsup src/index.ts --format esm,cjs --dts
�[34mCLI�[39m Building entry: src/index.ts
�[34mCLI�[39m Using tsconfig: tsconfig.json
�[34mCLI�[39m tsup v8.5.1
�[34mCLI�[39m Target: es6
�[34mESM�[39m Build start
�[34mCJS�[39m Build start
�[32mESM�[39m �[1mdist/index.mjs �[22m�[32m157.34 KB�[39m
�[32mESM�[39m ⚡️ Build success in 59ms
�[32mCJS�[39m �[1mdist/index.js �[22m�[32m165.24 KB�[39m
�[32mCJS�[39m ⚡️ Build success in 59ms
�[34mDTS�[39m Build start
�[32mDTS�[39m ⚡️ Build success in 3353ms
�[32mDTS�[39m �[1mdist/index.d.mts �[22m�[32m114.55 KB�[39m
�[32mDTS�[39m �[1mdist/index.d.ts �[22m�[32m114.55 KB�[39m
bangyro
commented
Apr 23, 2026
Collaborator
Author
There was a problem hiding this comment.
the first commit is just formatting with prettier, you can safely omit it from the review
bangyro
commented
Apr 23, 2026
Collaborator
Author
There was a problem hiding this comment.
using pnpm instead
bangyro
force-pushed
the
feat/sync_release_0.2.2
branch
2 times, most recently
from
e097b79 to
6585628
Compare
bangyro
force-pushed
the
feat/sync_release_0.2.2
branch
from
6585628 to
327f808
Compare
bangyro
commented
Apr 24, 2026
Collaborator
Author
There was a problem hiding this comment.
Only covering a few test cases for now. Might add more in the future
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.
Changes
sqrt_priceatsqrt_min_price) or fully token B (sqrt_priceatsqrt_max_price).Tests
Compounding Fee test
Note
For pool's that are near single-sided, zap-in is likely to fail if the amount of tokens to swap is too low. This happens because the jup quote function cannot find any routes to swap a small amount of tokens.
For example, in direct path for a pool composed of 99.9999% token A and 0.0001% token B. We might try to swap 1 lamport of token A to token B which is unlikely to find any swap routes.