Skip to content

Emit share_amount on lending events (closes #577)#583

Open
shubhamessier wants to merge 1 commit into
0dotxyz:mainfrom
shubhamessier:main
Open

Emit share_amount on lending events (closes #577)#583
shubhamessier wants to merge 1 commit into
0dotxyz:mainfrom
shubhamessier:main

Conversation

@shubhamessier
Copy link
Copy Markdown

Append share_amount: WrappedI80F48 to LendingAccountDepositEvent,
LendingAccountWithdrawEvent, LendingAccountBorrowEvent, and
LendingAccountRepayEvent. Indexers can now reconstruct share-based
balances from event history without re-deriving cumulative share price
or polling on-chain state.

Implementation:

  • New field is appended to each event struct (no reordering), preserving
    serialization compat for existing consumers per maintainer guidance in
    the issue thread.
  • Share delta is captured at each emit site by snapshotting
    bank_account.balance.{asset,liability}_shares before and after the
    deposit / withdraw / borrow / repay call. Because each public wrapper
    uses a single-direction Balance{Increase,Decrease}Type, the orthogonal
    share field is invariant across the call, so the snapshot is equivalent
    to the delta computed internally — no *_balance_internal signature
    change required.
  • Type is WrappedI80F48 (not u64 or f64) per maintainer comment:
    shares are fractional, f64 cast is CU-expensive, raw I80F48 preserves
    full precision.
  • Sign convention: positive value = shares moved (minted for
    deposit/borrow, burned for withdraw/repay). *_all paths produce the
    full pre-balance amount since post-close balance is zero.

Touched all event construction sites (12 total): the 4 in
instructions/marginfi_account/, plus juplend / drift / kamino / solend
deposit and withdraw, since they construct the same event types.

Verification:

  • cargo check -p marginfi --no-default-features --features custom-heap
    passes clean.
  • Anchor build / SBF build / basic-tests not run in dev environment
    (toolchain unavailable); CI should validate

@shubhamessier
Copy link
Copy Markdown
Author

Hello @jgur-psyops, can you kindly have a look at these changes once, thanks!

implementation per your guidance in #577:

Decisions you flagged:

  1. Append-only ordering: confirmed, all 4 events get the new field at the
    end. No existing field reordered.
  2. I80F48 vs f64: went with WrappedI80F48. Preserves full precision and
    avoids the CU hit you mentioned on the f64 cast. Happy to switch to
    f64 if downstream ergonomics outweigh that single-line change in
    events.rs plus .to_num() at the 12 emit sites.

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.

1 participant