Skip to content

Conversation

AminR443
Copy link
Contributor

@AminR443 AminR443 commented Sep 3, 2025

This PR is a step towards implementing the FFI iterator mentioned in #1009.

This implement FFI functions (fwd_iter_on_root, fwd_iter_on_proposal, fwd_iter_next) in Rust, and also necessary changes in firewood crate for MerkleKeyValueStream to work with Arc<NodeStore<..>> as well.

AminR443 and others added 30 commits July 29, 2025 17:29
/// The iterator is exhausted
None,
/// The next item on iterator is returned.
Some(OwnedKeyValuePair),
Copy link
Member

Choose a reason for hiding this comment

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

This is still a little confusing. The documentation is for this variant, but shouldn't the caller generally call fwd_free_owned_bytes regardless of which variant of KeyValueResult is returned, or must they check that it is this variant and only call that method if so?

@AminR443 AminR443 marked this pull request as draft September 24, 2025 16:02
Base automatically changed from brandon.leblanc/ffi-refactor to main September 26, 2025 17:21
@demosdemon
Copy link
Contributor

demosdemon commented Sep 26, 2025

idk your level of git foo, but here's how I normally resolve the gnarly merge conflicts from the squash and merge:

  • Find the commit from the base branch where I merged main before the squash and merge,
  • merge that specific commit into my child branch; this should be a clean merge unless there were conflicts between base branch and child branch
  • merge the specific squash commit in main with the ours strategy; this should be a net-zero change (no tree updates)
  • merge the rest of main

So, to resolve your conflicts with main now that my PR is merged, do:

# let git fetch the expired branch PR refs
git config --add remote.origin.fetch '+refs/pull/*/head:refs/remotes/origin/pr/*'
# fetch everything again
git fetch origin
git merge fcc07b8ba5e0716e3d8892a275010e77daaebbe5
git merge -s ours 8df1ccc1464f58839c732b57c6cd703e88137abc
git merge origin/main

The git config change will bloat your local repo. You can keep it if you want, but I do recommend removing it after, fetching again, and then running git prune and git gc to remove the bloat.

@AminR443 AminR443 changed the base branch from main to amin/ffi-revision October 2, 2025 08:42
@AminR443 AminR443 marked this pull request as ready for review October 2, 2025 10:05
@rkuris rkuris self-requested a review October 2, 2025 14:41
@AminR443 AminR443 requested a review from rkuris October 2, 2025 21:14
/// The iterator is exhausted
None,
/// The next item on iterator is returned.
Some(OwnedKeyValuePair),
Copy link
Member

Choose a reason for hiding this comment

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

This doc looks correct to me, but the comment is not resolved. Is there more to do here? If not, please mark this resolved.

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