-
Notifications
You must be signed in to change notification settings - Fork 0
fix state trie #366
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
fix state trie #366
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes primarily focus on a significant rework of the State Trie implementation to align with the JAM specification, including changes to node structure, hashing, and storage format. This also involves adding branch collapsing logic for deletions and updating tests accordingly. A potential issue was found where a message size limit was removed in the fuzzing socket, which could lead to resource exhaustion.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #366 +/- ##
==========================================
+ Coverage 81.64% 81.94% +0.29%
==========================================
Files 381 381
Lines 33572 33707 +135
==========================================
+ Hits 27410 27620 +210
+ Misses 6162 6087 -75 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found potential compile-time error in JIT executor, a removed safety check that can cause memory issues in fuzzing socket, brittle test expectation logic in TraceTest, and backward-compatibility risks from enforcing a new 65-byte trie-node storage format without migration handling.
Suggestions that couldn't be attached to a specific line
PolkaVM/Sources/PolkaVM/Executors/JIT/ExecutorBackendJIT.swift:192
The use of nonisolated(unsafe) on a local variable is invalid and will not compile. Additionally, the @Sendable Task closure writes to asyncResult, which violates concurrency safety. Replace the local var with a boxed reference that is @unchecked Sendable and write through the box inside the Task. For example: replace nonisolated(unsafe) var asyncResult: ExecOutcome? with let resultBox = UncheckedSendableBox<ExecOutcome?>(nil) and in the Task set resultBox.value = await boxedCtx.value.dispatch(...). After semaphore.wait(), read guard let asyncResult = resultBox.value else { ... }.
PolkaVM/Sources/PolkaVM/Executors/JIT/ExecutorBackendJIT.swift:198
The Task { @Sendable in ... } closure captures and mutates a local variable across concurrency. After applying the boxed approach above, update the closure to write to resultBox.value to satisfy sendability and avoid data races.
|
benchmark can be ignored for this one, it cannot build on master with latest swift version |
close #353