Skip to content

explicitly passing oracle initialSharedVersion in transaction#145

Merged
harshaalphafi merged 1 commit intomainfrom
feature/transactions
Apr 19, 2026
Merged

explicitly passing oracle initialSharedVersion in transaction#145
harshaalphafi merged 1 commit intomainfrom
feature/transactions

Conversation

@11felix
Copy link
Copy Markdown
Contributor

@11felix 11felix commented Apr 18, 2026

No description provided.

@11felix 11felix requested review from jangid and rg-alpha April 18, 2026 07:12
Copy link
Copy Markdown
Contributor

@jangid jangid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

1. Inconsistent oracle object references in oracle.ts (Bug)

updatePriceTransaction now uses Inputs.SharedObjectRef for the update_price_from_pyth call, but the second reference to the same oracle object in the get_price_info call still uses plain tx.object(constants.ALPHAFI_ORACLE_OBJECT_ID):

// explicit SharedObjectRef ✓
tx.object(Inputs.SharedObjectRef({
  objectId: constants.ALPHAFI_ORACLE_OBJECT_ID,
  initialSharedVersion: oracleInitialSharedVersion,
  mutable: true,
})),

// plain object reference ✗ (in get_price_info call)
tx.object(constants.ALPHAFI_ORACLE_OBJECT_ID),

If the explicit SharedObjectRef is needed to fix a resolution issue, the same fix should apply to both references. Otherwise one call works and the other may still hit the original problem.

2. Commented-out line left in oracle.ts

// tx.object(constants.ALPHAFI_ORACLE_OBJECT_ID),

Old line left commented out below the new SharedObjectRef call. Remove before merging.

3. Blockchain class is overly large for this PR's purpose

The new src/models/blockchain.ts adds ~200 lines including market queries, position queries, and position cap queries — none of which are used by this PR. Only getInitialSharedVersion() is called. This should either be a separate PR or trimmed to just what's needed here.

4. Network call on every transaction build

getInitialSharedVersion is called in both addUpdatePriceTransaction and addUpdatePriceTransactionForBorrow. While the Blockchain class caches results, these are await calls in transaction-building paths. Consider pre-fetching during AlphalendClient initialization since initialSharedVersion never changes for a shared object — this eliminates async overhead from every transaction build.

5. get_price_info may conflict with update_price_from_pyth

The get_price_info call reads the oracle via tx.object() which may auto-resolve as immutable, while update_price_from_pyth already declared it mutable via SharedObjectRef. The same object referenced twice with different mutability in the same PTB could cause a transaction conflict. Verify this doesn't cause issues.

6. Missing JSDoc update on updatePriceTransaction

The function signature now takes oracleInitialSharedVersion: string but the @param list wasn't updated.

7. Test script changes are noisy

Reducing amount, uncommenting withdraw(), and deleting 272 lines of dead test code are fine cleanup but unrelated to the oracle fix. Better as a separate commit to keep the diff focused.

@11felix
Copy link
Copy Markdown
Contributor Author

11felix commented Apr 18, 2026

  1. It is intentional. get_price_info doesn't need mutable reference
  2. This is to test if its fixes the bug so kept in case we need to revert
  3. No other new functions were introduced in blockchain except getInitialSharedVersion
  4. We cache in blockchain

Copy link
Copy Markdown
Contributor

@jangid jangid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Verified the mutability conflict concern — works fine. Minor: please remove the commented-out line before merging (use git history for reverts).

@jangid
Copy link
Copy Markdown
Contributor

jangid commented Apr 18, 2026

Thanks for the clarifications, Khush.

  1. Inconsistent oracle refs — Makes sense, get_price_info doesn't need mutable. Accepted.
  2. Commented-out line — Please remove before merging. Git history is there if you need to revert.
  3. Blockchain class scope — Fair enough, only getInitialSharedVersion is new. Accepted.
  4. Caching — Good, caching in Blockchain works. Accepted.
  5. Mutability conflict in PTB — Verified, works fine. No issue.
  6. Missing JSDoc — Please update the @param list when you get a chance.
  7. Test script changes — Minor, not blocking.

Approved!

@harshaalphafi harshaalphafi merged commit bc8df45 into main Apr 19, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants