Skip to content

Fix - bpf_account_data_direct_mapping #5764

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

Lichtso
Copy link

@Lichtso Lichtso commented Apr 11, 2025

Problem

Summary of Changes

This PR has a carefully curated commit history and the commit messages explain each refactoring step. Please review commit by commit in order.

Open TODOs for the new account_data_write_access_handler:

  • It is incorrect because in a rare reallocation pattern it can result in a dangling pointer which causes the test EXTEND_AND_WRITE_U64 of test_program_sbf_realloc to be flaky.
  • It should be optimized further to only allocate the realloc region if necessary, not if only the account data is touched.
  • It should be optimized even further by extending the account region into the virtual address space occupied by the realloc region so that each account only has one region carrying its payload and the realloc region is always uninitialized or non existent.
  • For all of the above it would need access to multiple memory regions at once and be region independent by getting a virtual address as parameter.

Open TODOs for the logic which decides when and how much Vec capacity to zero:

  • We might want to stop abusing the Vec::capacity and instead track an explicit additional field for each account.
  • The buggy way the realloc detection works needs to be completely rewritten.

There is a new behavioral change in this patch, which I don't expect programs to depend on, but it needs to be tested / measured on MNB. See:

  • SIMD-0219
  • Adjustment of TEST_CPI_ACCOUNT_UPDATE_CALLER_GROWS_CALLEE_SHRINKS.

Feature Gate Issue: https://github.com/anza-xyz/feature-gate-tracker/issues/16

Copy link

mergify bot commented Apr 11, 2025

The Firedancer team maintains a line-for-line reimplementation of the
native programs, and until native programs are moved to BPF, those
implementations must exactly match their Agave counterparts.
If this PR represents a change to a native program implementation (not
tests), please include a reviewer from the Firedancer team. And please
keep refactors to a minimum.

@Lichtso Lichtso force-pushed the fix/account_data_direct_mapping branch 7 times, most recently from 89a50f3 to a97e969 Compare April 13, 2025 20:20
@Lichtso Lichtso force-pushed the fix/account_data_direct_mapping branch from 2d5f262 to ea2a2b3 Compare April 16, 2025 14:59
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