Skip to content

Conversation

@demosdemon
Copy link
Contributor

@demosdemon demosdemon commented Aug 20, 2025

This is the final PR in the series to refactor the FFI layer to be more rust and also a bit more Gooey.

This replaces the serial atomic id for proposals with a pointer to the actual structures. The wrapper that includes the db handle is only necessary in order to do the cached view shenanigans. I will come back to that later.

Depends On: #1228

@demosdemon demosdemon changed the base branch from brandon.leblanc/issue-1051 to brandon.leblanc/ffi-refactor-p1 August 20, 2025 22:58
@demosdemon demosdemon added the DO NOT MERGE This PR is not meant to be merged in its current state label Aug 20, 2025
@demosdemon demosdemon changed the base branch from brandon.leblanc/ffi-refactor-p1 to brandon.leblanc/ffi-refactor-p2 August 20, 2025 23:48
@demosdemon demosdemon changed the base branch from brandon.leblanc/ffi-refactor-p2 to brandon.leblanc/ffi-refactor-p3 August 21, 2025 01:01
@demosdemon demosdemon changed the base branch from brandon.leblanc/ffi-refactor-p3 to brandon.leblanc/ffi-refactor-p4 August 21, 2025 01:48
@demosdemon demosdemon changed the base branch from brandon.leblanc/ffi-refactor-p4 to brandon.leblanc/ffi-refactor-p5 August 21, 2025 02:38
@demosdemon demosdemon changed the base branch from brandon.leblanc/ffi-refactor-p5 to brandon.leblanc/ffi-refactor-p6 August 21, 2025 03:00
@demosdemon demosdemon changed the base branch from brandon.leblanc/ffi-refactor-p6 to brandon.leblanc/ffi-refactor-p7 August 21, 2025 03:39
@demosdemon demosdemon changed the title feat(ffi-refactor): finish ffi refactor feat(ffi-refactor): finish ffi refactor (8/8) Aug 21, 2025
@demosdemon demosdemon removed the DO NOT MERGE This PR is not meant to be merged in its current state label Aug 21, 2025
@demosdemon demosdemon linked an issue Aug 21, 2025 that may be closed by this pull request
@demosdemon demosdemon changed the title feat(ffi-refactor): finish ffi refactor (8/8) feat(ffi-refactor): replace sequence id with pointer to proposals (8/8) Aug 21, 2025
@ava-labs ava-labs deleted a comment from github-actions bot Aug 21, 2025
@ava-labs ava-labs deleted a comment from github-actions bot Aug 21, 2025
@ava-labs ava-labs deleted a comment from github-actions bot Aug 21, 2025
@demosdemon demosdemon marked this pull request as ready for review August 21, 2025 04:50
@ava-labs ava-labs deleted a comment from github-actions bot Aug 22, 2025
@ava-labs ava-labs deleted a comment from github-actions bot Aug 22, 2025
@ava-labs ava-labs deleted a comment from github-actions bot Aug 22, 2025
@ava-labs ava-labs deleted a comment from github-actions bot Aug 22, 2025
@ava-labs ava-labs deleted a comment from github-actions bot Aug 22, 2025
@github-actions
Copy link

github-actions bot commented Aug 22, 2025

Metrics Change Detection ⚠️

This PR contains changes related to metrics:

-        counter!("firewood.ffi.propose_ms").increment(propose_time);
+            counter!("firewood.ffi.commit_ms").increment(commit_time.as_millis());
-        counter!("firewood.ffi.batch_ms").increment(propose_plus_commit_time);
-        counter!("firewood.ffi.commit_ms")
+        counter!("firewood.ffi.batch_ms").increment(start_time.elapsed().as_millis());
+            metrics::counter!("firewood.ffi.commit_ms").increment(commit_time.as_millis());
+            metrics::counter!("firewood.ffi.commit").increment(1);
+        counter!("firewood.ffi.propose_ms").increment(propose_time.as_millis());
+        counter!("firewood.ffi.propose").increment(1);

However, the dashboard was not modified.

You may need to update benchmark/Grafana-dashboard.json accordingly.


This check is automated to help maintain the dashboard.

@demosdemon
Copy link
Contributor Author

I'm gonna move this back to draft and get the coreth change ready first.

@demosdemon demosdemon marked this pull request as draft August 22, 2025 21:07
Base automatically changed from brandon.leblanc/ffi-refactor-p7 to main August 22, 2025 21:10
This is the final PR in the series to refactor the FFI layer to be more
rust and also a bit more Gooey.

This replaces the serial atomic id for proposals with a pointer to the
actual structures. The wrapper that includes the db handle is only
necessary in order to do the cached view shenanigans. I will come back
to that later.
@rkuris
Copy link
Member

rkuris commented Sep 12, 2025

Closes #1272

@rkuris rkuris linked an issue Sep 12, 2025 that may be closed by this pull request
@demosdemon
Copy link
Contributor Author

My coreth PR will be merged soon. This is r4r again.

@demosdemon demosdemon marked this pull request as ready for review September 15, 2025 18:39
We were incorrectly checking the length of the last node's full path
against the length of the key being proven. This meant that if the
last node's full path was the same _length_ as the key being proven, but
the key was different, we would incorrectly verify the proof by expecting
a value when there should be none.
…sion-proof' into brandon.leblanc/ffi-refactor
Base automatically changed from brandon.leblanc/serialized-range-proofs to main September 16, 2025 19:16
@demosdemon demosdemon merged commit 8df1ccc into main Sep 26, 2025
47 checks passed
@demosdemon demosdemon deleted the brandon.leblanc/ffi-refactor branch September 26, 2025 17:21
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.

bug(ffi): Creating a proposal every millisecond will fail after 50 days [ffi] Avoid overloading Value struct

4 participants