forward-allocating bump heap allocator #460

imef-femi · 2025-12-12T12:33:41Z

Summary:
Introduces a custom forward-allocating bump heap allocator with memory recycling capabilities, increasing MAX_KAMINO_POSITIONS from 8 to 16.

Key Changes:

New allocator.rs: Forward-allocating bump allocator with heap_pos()/heap_restore() for memory recycling between oracle price loads
Increased position limit: MAX_KAMINO_POSITIONS bumped from 8 → 16
Updated liquidation instructions: liquidate, liquidate_start, liquidate_end, and handle_bankruptcy now use heap-efficient health calculations
New test k18: Validates liquidation works correctly with 16 total positions (15 Kamino deposits + 1 borrow)

Why:
Previously, accounts with many Kamino positions risked becoming unliquidatable due to CU/heap memory exhaustion. The custom allocator recycles heap memory (~3KB per position) so 16 positions fit within 32KB without requiring requestHeapFrame.

Henry-E · 2025-12-12T18:15:47Z

We didn't really discuss this ahead of time. Not entirely sure this is the right solution here. It's quite a heavy change to make when we can support 14 out of 16 positions already.

Henry-E · 2025-12-12T18:18:42Z

There's a more comprehensive set of realistic tests of combinations of positions and mints and other things that @IliaZyrin identified that we need to test and get a realistic view of first before doing something like this imo.

jgur-psyops · 2025-12-13T20:35:17Z

programs/marginfi/src/allocator.rs

+## Why Backward Allocation Breaks with Extended Heap
+
+If we simply compile program with increased HEAP_LENGTH without changing direction
+and or not using `requestHeapFrame` (for functions that fit in 32 KiB), we get this situation:


this is interesting I didn't know the forward allocation was incompatible with requestheapframe, is there a demo or something to show this?

I've updated the doc to explain this better... but a similar implementation can be seen here:
Squads allocator.rs

programs/marginfi/src/state/marginfi_account.rs

jgur-psyops · 2025-12-13T20:40:48Z

programs/marginfi/src/state/marginfi_account.rs

+        let oracle_ais = &remaining_ais[oracle_ai_idx..end_idx];
+
+        // Create oracle adapter (heap allocation happens here)
+        let price_adapter_result = OraclePriceFeedAdapter::try_from_bank(&bank, oracle_ais, &clock);


did we experiment at all with how much heap the oracle call here actually uses in the kamino (like did we confirm it's 3kish bytes?)

maybe there is a way to cut back the heap usage?

the ~3k ish bytes is just an over-exaggeration (worst case)... after testing with logs, the usage usually doesn't exceed ~128 bytes (and thats for the switchboard oracles)

yeh it's fine if you're documenting worst case, just want to confirm the size at worst for our napkin math of memory usage.

If you can print the heap pointer before/after should be able to precisely doc the worst case.

imef-femi · 2025-12-14T07:28:07Z

There's a more comprehensive set of realistic tests of combinations of positions and mints and other things that @IliaZyrin identified that we need to test and get a realistic view of first before doing something like this imo.

wasn't aware we had that solution already @Henry-E ... @jgur-psyops mentioned it a while back and i just thought i'd give it a try

Henry-E · 2025-12-14T11:35:02Z

It's not a solution as such. It's more like there's a lot more factors to consider than just memory issues when scaling to >8 integration assets.

Things that would be a better use of your time than this

strip out anything that doesn't use bankrun and replace it with bankrun in the current typescript testing framework. The setup takes 40 - 60 seconds each time partly because of mocks program txes and then also some other tests that still use non-bank run testing. It would be much appreciated if you could just strip out and replace all that with bankrun transactions.
Once that's done and everything in the testing framework is on bankrun, that's when we can sit down and discuss what a proper comprehensive testing framework for scaling past 8 assets looks like. As mentioned earlier there's a few different aspects which need to also be tested which aren't being covered, e.g. different mints per bank. And also none of the integrations have tests with switchboard or the new liquidation approach. This is something we should sit down and discuss a plan for though.

jgur-psyops · 2025-12-14T20:32:54Z

Great implementation overall, will give it another look after I land. Once we confirm that 16 positions works with the custom heap reset (it seems to), let's see if we can cut memory down to get the same pass condition without resetting the heap pointer (the answer might be no, but I suspect it's possible).

don't spent too much time on it tho, if it's working with the custom heap then 1-2 days at most to try getting it working without.

…ing_bump_heap_allocator

…tion test

IliaZyrin · 2026-01-08T12:12:29Z

programs/marginfi/src/allocator.rs

+let checkpoint = heap_pos();
+{
+    let temp_vec: Vec<u8> = vec![1, 2, 3];          // Heap allocation
+    let sum: u64 = temp_vec.iter().sum();           // Copy to stack!


This is confusing comment, because we're not copying anything here, but rather putting a new value (sum) on stack.

IliaZyrin · 2026-01-08T15:26:03Z

programs/marginfi/src/instructions/marginfi_account/withdraw.rs

-        // Note: in flashloans, risk_engine is None, and we skip the cache price update.
-        maybe_price = risk_engine.and_then(|e| e.get_unbiased_price_for_bank(&bank_pk).ok());
+        // Fetch unbiased price for cache update
+        let bank = bank_loader.load()?;


So the logic is now exactly the same as in the else below. I suggest we just move it out the scope to not duplicate.

IliaZyrin · 2026-01-08T15:50:05Z

programs/marginfi/src/state/marginfi_account.rs

+        {
+            let heap_after_oracle = heap_pos();
+            let heap_used = heap_after_oracle.saturating_sub(heap_checkpoint);
+            msg!(


We should remove this before merging. Or, which is probably better, change it to debug!().

IliaZyrin · 2026-01-08T18:23:49Z

programs/marginfi/src/state/marginfi_account.rs

+    remaining_ais: &'info [AccountInfo<'info>],
+    health_cache: &mut Option<&mut HealthCache>,
+) -> MarginfiResult {
+    let (equity_assets, equity_liabs) = get_health_components(


Missing check for flashloan here? Also, should we just put it directly inside get_health_components if it's only used by the callers of this function?

IliaZyrin · 2026-01-08T18:26:55Z

scripts/build-program.sh

 fi

 if [ "$cluster" = "mainnet" ]; then
    features="--features mainnet-beta"


Not using custom-heap in mainnet?

IliaZyrin · 2026-01-08T18:29:39Z

tests/k14_limitsWithKamino.spec.ts


-    // Only deposit into the first MAX_KAMINO_POSITIONS banks
-    for (let i = 0; i < MAX_KAMINO_POSITIONS; i += depositsPerTx) {
+    // NOTE: We only deposit into 8 banks for this test suite since we don't use a LUT.


Let's just rename the const above to be KAMINO_POSITIONS and use it everywhere (with the value of 8, as before). Otherwise it creates confusion since MAX_KAMINO_POSITIONS is not even used.

IliaZyrin · 2026-01-08T18:30:45Z

tests/k17_kaminoLimitsMultiDeposit.spec.ts

    });

-    // Add compute budget
+    // Add compute budget       


IliaZyrin · 2026-01-08T18:34:25Z

tests/k17_kaminoLimitsMultiDeposit.spec.ts

-const MAX_KAMINO_DEPOSITS = 8; // Maximum Kamino positions per account
-const NUM_KAMINO_BANKS_FOR_TESTING = 9; // Create 9 banks to test liquidator limit
+
+const NUM_KAMINO_BANKS_FOR_TESTING = 17;


Do we really need 17 here? 15 is not enough?

Please update/delete the outdated comments throughout this file. E.g. they mention 9 banks and non-actual expectations.

IliaZyrin · 2026-01-08T18:37:21Z

tests/k18_sixteenKaminoLiquidation.spec.ts

+import { CONF_INTERVAL_MULTIPLE, ORACLE_CONF_INTERVAL } from "./utils/types";
+
+/** Maximum Kamino positions per account - now 16 with custom allocator */
+const MAX_KAMINO_POSITIONS = 16;


This is unused. Let's delete it.

I don't think we need a separate const for Kamino, given that the maximum Kamino positions is now equal to the maximum positions in general. But up to you, if you want to keep it for documentation purposes.

IliaZyrin · 2026-01-08T18:41:39Z

programs/marginfi/src/state/marginfi_account.rs

+/// with up to 16 positions.
+///
+/// Returns (account_health, assets, liabilities) if the account is liquidatable.
+pub fn check_pre_liquidation_condition_and_get_account_health<'info>(


General comment about all these new functions. Should we remove their non-heap-optimized versions now? Or is there a reason for both variants to stay?

forward-allocating bump heap allocator

d26a5d5

imef-femi requested a review from jgur-psyops as a code owner December 12, 2025 12:33

imef-femi requested review from Henry-E and IliaZyrin December 12, 2025 12:34

jgur-psyops reviewed Dec 13, 2025

View reviewed changes

programs/marginfi/src/state/marginfi_account.rs Outdated Show resolved Hide resolved

jgur-psyops reviewed Dec 13, 2025

View reviewed changes

imef-femi added 3 commits December 14, 2025 08:06

update readme docs

95bc972

add logs for HEAP_MEASURE

2f0cbed

cargo fmt

559f1ab

imef-femi added 4 commits December 23, 2025 01:06

refractor to make heap reuse the default behavior

926f8bd

Merge remote-tracking branch 'origin/0.1.7-main' into forward_allocat…

fc56a64

…ing_bump_heap_allocator

failing anchor test fix

8434390

increase compute budget for k17_kaminoLimitsMultiDeposit.spec liquida…

fe1c5ce

…tion test

IliaZyrin reviewed Jan 8, 2026

View reviewed changes

Uh oh!

forward-allocating bump heap allocator #460

Are you sure you want to change the base?

forward-allocating bump heap allocator #460

Conversation

imef-femi commented Dec 12, 2025

Uh oh!

Henry-E commented Dec 12, 2025

Uh oh!

Henry-E commented Dec 12, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

imef-femi Dec 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

imef-femi commented Dec 14, 2025

Uh oh!

Henry-E commented Dec 14, 2025

Uh oh!

jgur-psyops commented Dec 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

IliaZyrin Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

imef-femi Dec 14, 2025 •

edited

Loading

IliaZyrin Jan 8, 2026 •

edited

Loading