Repository navigation
fix: align SDK share conversions with vault withdrawal payout - #1181
Conversation
|
@temiloluwaolaoye7-rgb is attempting to deploy a commit to the Collins' projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@temiloluwaolaoye7-rgb Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
collinsezedike
left a comment
collinsezedike
left a comment
There was a problem hiding this comment.
The conversion change is the right direction, but the new test does not pass. computePosition returns 166.5, not 166: the raw ratio truncates at the stroop level, and 333e7 * 500e7 / 1000e7 is exactly 1,665,000,000 stroops, or 166.5 USDC. The comment at line 108 also says the vault burns total_assets; it burns adapter shares (total_adapter_shares). Update the expectation and the comment, and amend the commit subject to 72 characters or fewer.
| }); | ||
|
|
||
| describe("computePosition matches the vault withdrawal payout", () => { | ||
| // The vault burns adapter shares as `shares * total_assets / total_shares` |
There was a problem hiding this comment.
The vault's withdraw burns adapter shares, not total_assets: shares * total_adapter_shares / total_shares. If this comment is describing the helper's chosen raw share-price ratio, say that instead of claiming the contract uses total_assets here.
| principal: 333_0000000n, | ||
| entryTime: 0n, | ||
| }); | ||
| expect(rounded?.deposited).toBe(166); |
There was a problem hiding this comment.
This assertion fails in CI: the helper returns 166.5. The raw ratio truncates at the stroop level, and 333e7 * 500e7 / 1000e7 is exactly 1,665,000,000 stroops, which is 166.5 USDC. Update the expectation to 166.5, and correct the comment on line 140.
collinsezedike
left a comment
collinsezedike
left a comment
There was a problem hiding this comment.
@temiloluwaolaoye7-rgb Thank you for the contribution. If you would like to pick up another open issue, there are several available. Merging now.
eb84713 to
24e8137
Compare
|
@temiloluwaolaoye7-rgb If you found Meridian useful, please consider starring the repository. |
Overview
This PR resolves the parity defect between the SDK's share/asset conversion helpers and the vault's on-chain withdrawal payout. The vault's
withdrawburns adapter shares using the raw ratio (shares * total_adapter_shares / total_shares) with noVIRTUAL_OFFSET, whileconvertSharesToAssets/convertAssetsToSharesapplied the offset in both directions and claimed (in a doc comment) to match the withdrawal path. This change makes the two packages agree on a single documented formula and adds a test asserting the conversion against a known vault state, includingtotalAssets < totalShares.Related Issue
Changes
🔧 SDK Conversion Parity
[MODIFY]
packages/sdk/src/vault-base.tsconvertSharesToAssets/convertAssetsToSharesso the documented formula matches the vault's withdrawal payout (raw ratio, no offset in the withdrawal direction).[MODIFY]
packages/stellar-sdk-helpers/src/positions.tscomputePosition'sdepositedcalculation with the same documented formula used by the SDK helpers, so both packages report the same value for the same vault state.[ADD]
packages/stellar-sdk-helpers/src/positions.test.tstotalAssets < totalSharesedge case, and checks that the helper and SDK agree ondeposited.Verification Results
computePositionuse the same documented raw-ratio formula as the withdrawal burn; doc comment correctedtotalAssets < totalSharespackages/stellar-sdk-helpers/src/positions.test.tspnpm lint && pnpm typecheck && pnpm testpassCloses #1133