Skip to content

feat(oracle): add Pyth Lazer getters and event to interface - #16

Merged
rg-alpha merged 7 commits into
mainfrom
feature/pyth-core-upgrade
Aug 9, 2026
Merged

feat(oracle): add Pyth Lazer getters and event to interface#16
rg-alpha merged 7 commits into
mainfrom
feature/pyth-core-upgrade

Conversation

@preyam2002

Copy link
Copy Markdown
Contributor

Mirrors the Pyth Lazer surface from alphalend-contracts#151 into the published interface package: the LazerPriceUpdationEvent struct, the get_conf reader, and the is_lazer_enabled / get_lazer_feed_ids_for_coin / get_coin_type_for_lazer_feed_id config getters.

Stub bodies, matching the existing interface convention (consumers link the real package at publish). Additive and flag-gated upstream — no consumer behavior changes until Lazer is enabled.

Mirrors the Pyth Lazer surface from alphalend-contracts#151 into the published interface: the LazerPriceUpdationEvent struct, the get_conf reader, and the is_lazer_enabled / get_lazer_feed_ids_for_coin / get_coin_type_for_lazer_feed_id config getters. Stub bodies, matching the existing interface convention.
Adds the Lazer writer stub parallel to update_price_from_pyth, taking pyth_lazer::update_v2::Update. Vendors the pyth_lazer ABI stub (types-only, mirroring alphalend-contracts#151) so the interface exposes the same signature consumers link against.
…et stub variant

F1: completes the Lazer getter surface (real oracle exposes it). F2: pyth_lazer stub now carries an explicit active-mainnet + commented-testnet published-at, matching the Pyth/Wormhole dep convention so the testnet republish has a swap point.
@rg-alpha
rg-alpha requested review from Zorag44 and rg-alpha July 1, 2026 07:37
@jangid

jangid commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

<!-- operate.md-review -->
operate.md security review — Move interface audit (new findings only)

Verified the stub types (I64/I16/Update/Feed, abilities, and all consumed getter signatures) against upstream pyth-network/pyth-crosschain lazer/contracts/suiABI-correct. Cross-package linking resolves by module+function+type identity, and the missing store on Update/Feed correctly forces by-value consumption in the PTB (good). New items, all LOW/INFO:

🔴-to-verify (INFO, load-bearing) — published-at / type-origin binding (deps-mainnet/lazer_interface/Move.toml).
published-at = 0xefbf…ee10 (claimed v2, must contain update_v2/feed) with type-origin pyth_lazer = 0x7b50…f580 (claimed v1 original). This is the single most security-relevant line: if either ID is wrong, the consumer either fails to link or silently links a pyth_lazer::update_v2::Update from an incorrect origin, feeding ingest_lazer_update a forged-type Update. I couldn't verify these on-chain from here. Please confirm both IDs against the on-chain package before publish, and that original-published-id in the generated Move.lock matches 0x7b50…f580.

🔵 Low — conf field semantics diverge between events (indexer footgun).
LazerPriceUpdationEvent.conf = real spot confidence band, but the sibling PythPriceUpdationEvent.conf = vestigial constant 0. Off-chain consumers subscribing to both get two meanings for an identically-named/typed conf. Confirm the indexer distinguishes by event type, not field name.

🔵 Low — self-referential doc comment.
The new event's doc says "Mirrors alphafi_oracle::oracle::LazerPriceUpdationEvent" — it mirrors itself; it should point at the real source-of-truth impl (per #151) so the field-sync discipline governing this ABI has a correct pointer.

Nits: edition = "2024.beta" in the stub vs upstream "2024" (harmless, inconsistent); add an explicit comment that Update/Feed must never gain store so a future re-sync doesn't break the verify-in-PTB model.

Automated review; no approval implied. Merge decision remains with the maintainers.


operate.md 2026-07-11 — status: SECURITY-CLEAN, recommend approval (gated)

Re-derived on head bfe9bf5: all prior items above are resolved (edition, conf semantics, self-referential doc). Signatures are byte-identical to #151; Feed/Update carry copy, drop only (no store), preserving the by-value-consumption model, now with an explicit "NEVER add store" comment. APPROVED by @rg-alpha + @Zorag44 (2026-07-10). Verdict: SECURITY-CLEAN. The only remaining item is operational, not code: re-confirm the published-at / original-published-id of the pyth_lazer deps on-chain at publish (the load-bearing INFO item above). ✅ Recommend approval on your OK per the contract-family gate.

Automated review; no approval implied.

@preyam2002

Copy link
Copy Markdown
Contributor Author

Addressed in bfe9bf5.

  • Verified mainnet binding: published-at = 0xefbf...ee10 is package v2, contains update_v2/feed, and exposes parse_and_verify_le_ecdsa_update_v2.
  • Verified normalized update_v2/feed modules and Update/Feed types resolve to original package id 0x7b50...f580; Update and Feed have copy, drop only, no store.
  • Updated deps-mainnet/lazer_interface/Move.toml to edition = "2024" and documented the published-at/original-id binding.
  • Clarified LazerPriceUpdationEvent.conf vs PythPriceUpdationEvent.conf so indexers branch by event type.
  • Fixed the self-referential event comment and added explicit “never add store” comments for Update/Feed.

@jangid jangid left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

operate.md review — Lazer interface mirror

ABI verified clean against the source of truth and consumers:

  • interface.move is byte-identical to alphalend-contracts#151.
  • Every consumed signature matches #151 exactly — LazerPriceUpdationEvent fields, get_conf, ingest_lazer_update(&mut Oracle, LazerUpdate, &Clock), and all four Lazer getters.
  • Consistent with the SDKs: alphalend-sdk-rust#207 and alphalend-sdk-js#155 both call oracle::ingest_lazer_update and invoke parse_and_verify_le_ecdsa_update_v2 against the real deployed Lazer package id — so the stub correctly ships types only and never links the verifier (survives Pyth verifier upgrades without republish).

Remaining, non-ABI:

  • 🟡 Regenerate/commit alphafi_oracle/Move.lock — the new [dependencies.pyth_lazer] in Move.toml isn't reflected in the committed lock.
  • 🟡 Add deps-mainnet/lazer_interface/Move.lock — every other mainnet dep (and #151) commits one; it's the artifact that pins original-published-id = 0x7b50….
  • 🟢 Consider mirroring LazerPriceSkippedEvent too (#151 ships both Lazer events).
  • 🟢 Harmonize the lazer_interface/Move.toml type-origin comment with #151's wording (0xefbf introducing-version vs 0x7b50 canonical id — no ABI impact, just avoid contradictory guidance on a future re-sync/on-chain check).

rg-alpha
rg-alpha previously approved these changes Jul 10, 2026
Zorag44
Zorag44 previously approved these changes Jul 10, 2026
@jangid

jangid commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

operate.md review — new finding (deduplicated vs existing approval)

✅ ABI stub verified byte-identical to the impl in alphalend-contracts#151: LazerPriceUpdationEvent { coin_type, feed_id, price, ema_price, conf, last_updated }, all getter signatures, and ingest_lazer_update match; published-at and pyth_lazer addresses match. Pure abort 0 stub, no fund path.

🟢 [LOW] Interface omits LazerPriceSkippedEvent. #151 emits LazerPriceSkippedEvent on every skip of a tracked feed (reasons 0–13) as the primary Lazer observability signal, but this interface package (which downstream SDKs/indexers link for ABI/event decoding) only exposes LazerPriceUpdationEvent. Post-cutover an indexer built against this interface can't decode skip events and will be blind to why a feed stopped updating (breaker/stale/conf-band). Recommend adding the LazerPriceSkippedEvent struct here for parity.

jangid
jangid previously approved these changes Jul 15, 2026

@jangid jangid left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

operate.md: approved per CTO authorization — pure ABI stub (abort 0 bodies, no fund path), verified byte-identical to the impl in alphalend-contracts#151. Non-blocking LOW already noted above: add LazerPriceSkippedEvent for indexer decode-parity.

@jangid

jangid commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

operate.md review — [move-audit], re-derived at current head (feature/pyth-core-upgrade)

This replaces the earlier summary at this marker. The Move ABI surface was re-verified symbol-by-symbol against alphalend-contracts#151 with a real toolchain build. Everything previously raised on this PR (ABI byte-identity, Update/Feed missing store, conf semantics, the self-referential doc, edition, the LazerPriceSkippedEvent omission, Move.lock churn, published-at re-confirmation) is either resolved or explicitly maintainer-decided, and none of it is re-filed here.

Newly verified clean: LazerPriceSkippedEvent in alphafi_oracle/sources/oracle.move is byte-identical to #151 — same field order and abilities. It was added in 696aa74, after the last symbol-by-symbol pass, so it had not previously been checked. All new getters take &Oracle / &PriceInfo only: no capability returned, no store object returned by value, no new PTB attack surface. The interface Oracle layout correctly stays unchanged, because #151 stores Lazer config in a dynamic field (LAZER: u64 = 2) rather than a new struct field.

No fund path, no capability leak, no type confusion in the Move surface itself. The findings below are dependency-pinning and CI hygiene.

🟡 sui_system is pinned to a moving branch, with override = true and no committed lock

alpha_lending/Move.toml:

sui_system = { git = "https://github.com/MystenLabs/sui.git", rev = "mainnet", subdir = "crates/sui-framework/packages/sui-system", override = true }

rev = "mainnet" is a branch, not a tag or commit. override = true makes it the winner for the entire transitive graph, displacing the SuiSystem that Pyth→Wormhole and Bridge pull in. Combined with 1b7ff65 "chore: drop generated move lockfile churn", nothing pins it: every fresh resolve re-fetches whatever mainnet points at that day, with no diff to review. A resolve during this review landed on 433212f8f2768ac22dd412229c970b8a5281d4e5; a later one gets something else, silently.

The change itself is necessary — base main genuinely fails to build (alpha_lending/sources/alpha_lending.move uses sui_system::sui_system::SuiSystemState, and sui move build --build-env mainnet --path alpha_lending on origin/main returns a compilation error). So the fix is right; only the pinning is wrong. The house convention is already in the file, four lines below, in the commented-out block: rev = "mainnet-v1.49.2". Suggest matching it (or a full SHA) and committing the resulting lock.

Framework version skew here changes the bytecode that alpha_lending's published-at consumers verify against, and CI cannot catch a moving pin by construction.

🟡 The pyth_lazer dep is hardcoded to the mainnet stub — a testnet build silently binds a mainnet type identity

alphafi_oracle/Move.toml points pyth_lazer at local = "../deps-mainnet/lazer_interface" for every environment, and deps-testnet/lazer_interface does not exist. Confirmed by listing both trees at this head: deps-mainnet/ has lazer_interface; deps-testnet/ does not — while every other vendored dep (cetus, clmmpool, mmt_v3, move-stl, integer-mate, bluefin-spot-…) exists in both.

The stub's own comment states this should fail loudly:

#   TESTNET: Lazer is STILL v1 (no update_v2 module) at 0xf5bd…8c21 — a testnet build will FAIL to link

It does not fail. sui move build --build-env testnet --path alphafi_oracle exits 0, and the generated lock shows why:

[pinned.testnet.pyth_lazer]
source = { local = "../deps-mainnet/lazer_interface" }
use_environment = "testnet"

The testnet build resolves the mainnet stub and inherits [addresses] pyth_lazer = "0x7b50…f580" — the mainnet original package id, which is the address baked into the update_v2::Update type identity in the emitted bytecode. An oracle published on testnet would then expect 0x7b50…f580::update_v2::Update, which no testnet Lazer package can ever produce, making ingest_lazer_update permanently uncallable there — a silent dead path, not a link error.

Compounding it, the swap instructions in that file tell the operator to change only published-at; they never mention that [addresses] pyth_lazer is the type-origin address and must change too. Following the documented procedure verbatim produces exactly the broken binding.

Suggest adding deps-testnet/lazer_interface/ with the testnet original id (mirroring the repo convention) and selecting the dep path by environment. At minimum, correct the two comment blocks — the "will FAIL to link" claim is empirically false, and the swap list must include [addresses] pyth_lazer.

🟡 CI installs its toolchain via unpinned curl | sh with GITHUB_TOKEN in scope and no permissions: block

.github/workflows/ci.yml:

      - name: Install suiup
        run: curl -sSfL https://raw.githubusercontent.com/MystenLabs/suiup/main/install.sh | sh
      - name: Install Sui CLI
        run: ~/.local/bin/suiup install sui@${SUI_VERSION} --yes
        env:
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

Three compounding issues:

  1. The installer comes from MystenLabs/suiup's main branch — not a tag or release, no checksum — piped straight into sh. Whatever it writes to ~/.local/bin/sui runs with secrets.GITHUB_TOKEN in the environment, and via upload-artifact/download-artifact is re-executed in all three downstream jobs, which do check out the repo. (To be precise about the blast radius: the install-sui job itself has no actions/checkout, so PR-authored code is not what executes there — the exposure is to the third-party installer, not to this PR's contents.)
  2. The workflow declares no permissions: key at all, so the token inherits the repo/org default scope rather than contents: read.
  3. SUI_VERSION: ${{ vars.SUI_VERSION || 'mainnet' }}mainnet is a moving channel. Since build/lint/test are the required checks for this contract family's ruleset, the gate itself is non-reproducible: a Sui release can flip it green→red or red→green with zero repo change.

Suggest permissions: contents: read at workflow level, pinning the installer to a tag URL with a checksum (or a prebuilt release asset), pinning SUI_VERSION to an explicit mainnet-vX.Y.Z, and dropping GITHUB_TOKEN from the install step unless a rate limit forces it. This org has already had one supply-chain incident through an install-time hook, which is why this is worth doing rather than noting.

🟢 CI only builds --build-env mainnet, so the finding above is structurally invisible

Every build step passes --build-env mainnet. The repo maintains a full deps-testnet/ tree and the stub ships a commented testnet swap point, but no job ever builds testnet — so the missing deps-testnet/lazer_interface and the mainnet-address bleed-through can never be caught. The workflow also triggers only on pull_request and workflow_dispatch, with no push: branches: [main], so main is never validated post-merge — which matters given the moving sui_system pin can break it with no repo change. A testnet matrix leg and a push trigger would cover both.

🟢 OracleLazer and the LAZER key are not mirrored into the interface

#151 introduces public struct OracleLazer has store, key { id, coin_to_identifier, identifier_map }, attached via dynamic_field::add(&mut oracle.id, LAZER, …) with const LAZER: u64 = 2. This package mirrors the events, getters, and writer — but not the OracleLazer type identity or the LAZER key. Downstream Move packages and SDK/indexer codegen linking this interface therefore cannot name or resolve the new dynamic field, nor decode the object. Same omission class as the LazerPriceSkippedEvent gap already fixed here.

🟢 The reason code space is undocumented in the package indexers actually read

LazerPriceSkippedEvent carries a bare reason: u8 with no comment, while LazerPriceUpdationEvent directly above it has a full explanatory block. #151 documents an authoritative 15-value table (0 = no spot price … 9 = stale, 10 = circuit breaker, 11 = non-monotonic ts, 12 = clock skew, 14 = untracked-at-emit) right above the struct. Since this interface package is exactly what SDK and indexer authors consume for event decoding, an opaque u8 here defeats the observability purpose the event exists for. Copying the table across costs nothing and keeps the two repos re-syncable.

🟢 Zero-warning policy is not enforced by the new lint job

Measured: origin/main produces 34 compiler warnings, this head 37. The three new ones are W09002 unused variable for self, update, clock in ingest_lazer_update — consistent with the existing update_price_from_pyth convention, so not a regression in style. But sui move build --lint exits 0 with all 37 present, so the new required lint check does not enforce the stated zero-warning rule. Either add a warning-count gate or prefix the stub params with _ as the compiler suggests.

Standing

Approvals: 1/2. No approval posted — this is a contract PR, so approval is left to a human reviewer by policy. On the merits: the ABI is correct and nothing here blocks the interface landing. The dependency-pinning and CI items should be fixed before the Lazer stack is published, which is a different gate than this merge.

@jangid

jangid commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

operate.md review — ✅ approvable; contract-adjacent → merge gated

Status re-derived this run: mergeable=MERGEABLE, mergeStateStatus=BLOCKED (pending required review count, not a conflict), reviewDecision=APPROVED with 3 approvals (jangid, Zorag44, rg-alpha) — threshold is 2, so the review bar is met.

Merge-order note (this is part of the Pyth Lazer stack): this interface package declares the new Lazer getters/event that alphalend-contracts#151 implements against, so it should land in coordination with #151 — but #151 itself is currently BLOCKED on the unresolved oracle-staleness decision (see that PR). Recommend holding the whole Lazer stack — including this interface merge — until the max_age/staleness_threshold margin is set, so the interface and its implementation move together and nothing half-lands. This PR touches interface definitions on contract-adjacent surface, so the merge is CTO-gated regardless.

@preyam2002
preyam2002 dismissed stale reviews from jangid, Zorag44, and rg-alpha via 696aa74 July 21, 2026 17:33

@jangid jangid left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

operate.md: approving on explicit maintainer authorisation for this contract PR (contract PRs are otherwise held for a human glance).

Re-derived at HEAD 1cfc635: CI green on all four contexts (install-sui, build, lint, test), MERGEABLE/BLOCKED (pending review, not a conflict). Zorag44's approval (2026-07-23T11:23Z) post-dates HEAD (2026-07-21T18:12Z), so it is current — this makes it 2/2.

The Move ABI surface was audited symbol-by-symbol against alphalend-contracts#151 on 2026-07-31, at a point newer than this PR's head, and the head has not moved since: LazerPriceSkippedEvent byte-identical to #151, all new getters take &Oracle / &PriceInfo only (no capability returned, no store object returned by value, no new PTB attack surface), and the interface Oracle layout correctly stays unchanged because #151 keeps Lazer config in a dynamic field. Nothing in that audit is re-litigated here.

Remaining open items are 🟡 dependency-pinning and CI hygiene (notably sui_system pinned to the moving 'mainnet' branch with override = true and no committed lock) — non-blocking, worth closing separately.

Approvals: 2/2 — merge-ready.

@rg-alpha
rg-alpha merged commit e541239 into main Aug 9, 2026
4 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