WL-777 waterx sdk increase decrease wlp view + test#11
Conversation
- Trading: AccountRequest via bucket framework; deposit/withdraw collateral; increase/decrease position; destroy_response 3 type args; keeper APIs - Orders/WLP: match contract signatures (recipient, cancel sender) - Fetch: getWlpTotalSupply, getWlpTvlUsd - README: concise exported API reference for temporary branch work Made-with: Cursor
Made-with: Cursor
…increase/decrease tx builders Made-with: Cursor
- Add unit/simulate/integration/e2e tests; import tx-builders and isValidReferralCode via src subpaths where not re-exported from index. - Align src with origin/main; remove extractVaa-only unit tests (helper is non-exported on main). Drop unused imports in client and tx-builders. - Restrict Prettier to test/ and scripts/ so SDK layout stays unchanged.
Made-with: Cursor
Made-with: Cursor
- CLAUDE.md: reflect new lint toolchain (ESLint + Prettier) and multi-project test scripts (test:unit, test:e2e, test:integration, test:ci, lint:eslint, lint:prettier, lint:fix) - test/integration/setup.ts: log a console.warn per failed keystore entry instead of silently continuing, to aid debugging
…EADME - Resolve view module package from shared object runtime type; market/position views use <LP> only - Align MarketSummary, PositionInfo, TokenPoolSummary BCS with deployed view.json - Testnet WLP type to wlp_token::WLP_TOKEN; WLP_STANDALONE for contracts/wlp package - Update PTB invariant tests (3 type args, match_orders indices, deposit/withdraw collateral) - tsconfig: noEmit + allowImportingTsExtensions for root typecheck - README: document view behavior and API changes (marketIndex, lastPriceRefreshTimestamp) - query-output: replace getMarketCount with WLP supply/TVL dumps Made-with: Cursor
- Add shared recipient/coin IDs (mockSuiAddress) and dummyBucketFloatPriceResult - Wire user-wlp and tx-builders WLP tests; cross-link from sui-mock-fixtures Made-with: Cursor
- Add PTB_DUMMY_ACCOUNT_ID, PTB_DUMMY_RECEIVING_COINS, dummyBucketFloatPricePair - Align user-order-ptb imports with .ts extension Made-with: Cursor
- Trigger CI on pull_request to main only; one Lint job (pnpm lint), Test uses test:ci - Remove lint.yml and unit-test.yml to avoid double eslint / double vitest - Document in test/README; keep security + coverage jobs Made-with: Cursor
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
- Format ptb-test-dummies.ts for Prettier (lint job) - Pass workflow inputs via env in publish.yml (Semgrep run-shell-injection) - Add .gitleaks.toml allowlist for public Sui IDs in constants.ts, dist/, coverage/ Made-with: Cursor
bucket-bot
left a comment
There was a problem hiding this comment.
Code review
No issues found. Checked for bugs and CLAUDE.md compliance.
Notes from spot-check:
- PTB flow matches latest request/execute/destroy_response pattern with AccountRequest.
- view package resolution + updated BCS layouts align with the stated on-chain changes.
- Newly added tests cover key PTB wiring and fetch parsing paths.
🤖 Generated with Rex (OpenClaw)
bucket-bot
left a comment
There was a problem hiding this comment.
Code review
Found 2 issues:
1. N+1 RPC calls from viewPackageForObject — every view call now fetches the shared object type first
getAccountsByOwner, getAccountDelegates, getAccountObjectId each call viewPackageForObject(client, client.config.accountRegistry) — resolving the same object ID on every invocation. Same for getMarketSummary / getPosition / positionExists (each resolving the market object), and pool queries (resolving wlpPool). Without memoization this doubles latency on every read.
Suggestion: cache resolved package IDs at the call site (e.g. a module-level Map<objectId, packageId> or inside WaterXClient) — the type of a shared object doesn't change unless deliberately upgraded, so a session-scoped cache is safe.
Lines 145 to 157 in 99e7093
2. buildMintWlpTx and buildSettleRedeemWlpTx are now async — silent breaking change for callers
Both functions changed from sync to async. Any caller that was using the return value without await (e.g. directly passing to signAndExecuteTransaction) will silently pass a Promise<Transaction> instead of a Transaction. TypeScript will catch this if callers are typed, but if any downstream code uses these via the SDK without strict type-checking, it will silently break at runtime. Worth a clear callout in the release notes or a CHANGELOG entry.
Lines 661 to 667 in 99e7093
Lines 750 to 757 in 99e7093
Everything else looks solid:
- Contract alignment (3 type args for
destroy_response,deposit_collateral/withdraw_collateralrenames,accountSenderRequestwiring,recipientonrequestRedeemWlp) is consistent throughout - BCS fix (
lp_decimal: bcs.u8()) is correct — the oldu64would have silently corrupted all subsequent fields viewPackageForObjectpattern is the right approach for handling package upgrades between shared objects- Testnet constant updates (GLOBAL_CONFIG, ACCOUNT_REGISTRY, REFERRAL_TABLE, WATERX_PERP) look consistent
- CI consolidation (replacing
lint.yml+unit-test.ymlwith singleci.yml) is cleaner; Semgrep shell injection fix (inputs via env) is correct - Test infrastructure (multi-project Vitest, coverage-v8, PTB assertion helpers) is well-structured
📦 Dependency Security
New devDeps: @vitest/coverage-v8, dotenv, typescript-eslint, eslint-config-prettier, globals — all well-established packages, no advisories or concerns.
🤖 Generated with Jarvis (OpenClaw)
bucket-bot
left a comment
There was a problem hiding this comment.
Code review
No issues found. Checked for bugs and CLAUDE.md compliance.
Notes from spot-check:
- trading PTB flow matches latest request/execute/destroy_response pattern with AccountRequest.
- fetch view package resolution + updated BCS layouts align with the stated on-chain changes.
- Newly added tests cover key PTB wiring and fetch parsing paths.
🤖 Generated with Rex (OpenClaw)
.github/workflows/ci.yml
Outdated
| runs-on: ubuntu-latest | ||
| name: Build | ||
| steps: | ||
| - uses: actions/checkout@v4 |
…p, upload-artifact) Made-with: Cursor
bucket-bot
left a comment
There was a problem hiding this comment.
Follow-up review (latest contract alignment: naming & params)
整體對齊方向是對的,不過有幾個命名/參數面向建議再收斂,避免 SDK 使用端誤解:
-
Builder async breaking change 需要明確標示
buildMintWlpTx、buildSettleRedeemWlpTx已改成async(回傳Promise<Transaction>)。- 建議在 README / changelog 明確標記 breaking change,並在 migration snippet 全部補
await。
-
accountId命名語意偏弱,建議改成accountObjectAddress(或userAccountAddress)- 多處實際是
tx.pure.address(...)且語意是 UserAccount object address。 - 目前
accountId容易讓人誤以為是 numeric id / registry index。
- 多處實際是
-
保留舊 API 名稱可理解,但應明確 deprecate
increaseCollateral/releaseCollateral實際已對應deposit_collateral_request/withdraw_collateral_request。- 建議:
- 新增對外 alias:
depositCollateral/withdrawCollateral - 舊名稱加
@deprecated註記 + 移轉說明。
- 新增對外 alias:
-
ViewMarketTypeArgs包含baseTokenType但目前被忽略,容易誤導- 若現階段只吃 LP type,建議先移除
baseTokenType或在型別名稱/註解明確標示 ignored。
- 若現階段只吃 LP type,建議先移除
-
viewPackageForObject建議加 cache(效能)- 現在每次 view call 都會
getObject解析 package。 - 可在
WaterXClient或 module scope 做Map<objectId, packageId>快取,減少重複 RPC。
- 現在每次 view call 都會
以上都偏 API ergonomics / DX 層級,非 blocker。
🤖 Generated with Rex (OpenClaw)
src/tx-builders.ts
Outdated
| feed: PYTH_TESTNET_FEED_IDS["ETH/USD"]!.replace(/^0x/, ""), | ||
| }; | ||
| } | ||
| throw new Error(`No Pyth feed mapping for token type: ${tokenType}`); |
There was a problem hiding this comment.
目前接 Hermes 只有 config 裡那幾個、若要再支援其他幣種,需要再補 mapping 或改用低階方式自己帶價格
src/tx-builders.ts
Outdated
| const cfg = client.config; | ||
|
|
||
| increasePosition(client, tx, { | ||
| collateralTokenType: cfg.usdcType, |
There was a problem hiding this comment.
高階 build*Tx 這邊加上可選的 collateralTokenType,不填的話預設還是 USDC
src/tx-builders.ts
Outdated
| const cfg = client.config; | ||
|
|
||
| decreasePosition(client, tx, { | ||
| collateralTokenType: cfg.usdcType, |
| tx.object(client.config.globalConfig), | ||
| tx.object(client.config.accountRegistry), | ||
| tx.object(params.market), | ||
| tx.pure.id(params.accountId), |
There was a problem hiding this comment.
不要,公開API一律改為 accountObjectAddress(UserAccount 的物件 id 字串)
Made-with: Cursor
…ses, view cache - README: breaking change for async buildMintWlpTx/buildSettleRedeemWlpTx; accountId semantics - trading: depositCollateral/withdrawCollateral as primary; deprecate increase/release - tx-builders: buildDepositCollateralTx/buildWithdrawCollateralTx; deprecate old names - fetch: ViewMarketTypeArgs only lpTokenType; module Map cache for viewPackageForObject - Tests: stub getObject for view helpers; use new builder/trading names - CLAUDE.md: document preferred collateral API names Made-with: Cursor
…order accountObjectAddress - addPriceFeeds/fetchAccountCoins use configurable collateral type (default usdcType) - resolveTokenPriceFeed: document scope; clearer error for unmapped types - PlaceOrderParams/CancelOrderParams: accountId -> accountObjectAddress (breaking) - BuildPlaceOrderParams/BuildCancelOrderParams: accountObjectAddress + optional collateralTokenType - README/CLAUDE: Allen review (pairs scope, collateral override, naming) Made-with: Cursor
- Rename public accountId fields to accountObjectAddress (trading, account, tx-builders, AccountInfo, fetch helpers) for consistent naming with orders. - Document pure.address vs pure.id usage in README and CLAUDE.md. - Add PTB_DUMMY_ID_CC / PTB_DUMMY_OBJECT_DD; reuse dummies in unit tests. Made-with: Cursor
- withdrawCollateral / buildWithdrawCollateralTx: primary collateralAmount, deprecated amount. - parseTokenPoolSummaryReturn: legacy 72B + optional 8B tail, or vNext BCS (u8 decimal + timestamp). - Mark getAccountObjectId @deprecated; README/CLAUDE + simulate test use accountObjectAddress. Made-with: Cursor
Summary
Delivers WL-777 SDK work: trading helpers for increase/decrease position, WLP-related reads and tx builders, testnet-aligned
fetch(view package resolution + BCS matching deployedviewstructs), plus Vitest unit/e2e (and integration project), coverage, and README/tsconfig updates.Details
increasePosition/decreasePositionand high-levelbuildIncreasePositionTx/buildDecreasePositionTx; WLP mint/redeem paths updated for current params (e.g. oracle,recipientwhere required).view::*package from shared object runtime type;market_summary/position_*use<LP>; alignMarketSummary,PositionInfo,TokenPoolSummaryBCS with on-chain layout; testnetWLPtype matches live pool (wlp_token::WLP_TOKEN).match_orders, deposit/withdraw collateral names), e2e simulate tests,query-outputscript (WLP supply/TVL dumps).tsconfigforsrc+test+scriptstypecheck;pnpm test:coverage/test:ciunchanged and valid.Supersedes
Closes the older tests-only PR (e.g.
feat/sdk-tests-and-ci) in favor of this branch, which includes those changes plus SDK + fetch fixes.How to verify
pnpm install && pnpm typecheck && pnpm test && pnpm test:coverage