-
Notifications
You must be signed in to change notification settings - Fork 0
GP 0.7.2 #376
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
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.
Found 3 behavioral regressions in the host-call and guaranteeing logic.
Suggestions that couldn't be attached to a specific line
Blockchain/Sources/Blockchain/VMInvocations/HostCall/HostCalls.swift:1445,1541-1543
The Log host call now unconditionally writes HostCallResultCode.WHAT into register 7 after emitting the message. In the GP’s host-call result table (see ResultConstants.swift), WHAT represents “name unknown” and signals failure. Returning WHAT on every successful log means services can no longer distinguish success from an actual WHAT error. The call should report HostCallResultCode.OK (or propagate a real error) when the log is accepted so that callers can observe the true status.
Optimizes the accumulation statistics calculation by computing the digests array once before iterating through services, rather than recomputing it inside the loop.
Calculate the set of reporters from extrinsic guarantees in `StatisticsTests` instead of passing an empty array. This allows removing the `withKnownIssue` wrapper from `tinyTests`.
Add missingWorkResults error to GuaranteeingError and validate that work reports contain digests before processing.
- Update `encode` to return a systematic codeword (original + parity) and correctly calculate parity count. - Add padding support in `chunk` and padding removal in `reconstruct`. - Optimize `reconstruct` with a fast path when all original shards are available. - Enable `tinyTests` and `fullTests` in `ErasureTests`.
- Refactor `PreimageStatusMapEntry` to `PreimageRequestMapEntry`, incorporating `length` into the key. - Update `Output` enum to support error codes and switch to unkeyed container encoding. - Modify `FullAccumulateState` to index preimage info by `HashAndLength`. - Adjust test setup and assertions to align with the updated data structures.
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 off-by-one errors in gas limit checks and an optimization opportunity for statistics calculation.
Suggestions that couldn't be attached to a specific line
Blockchain/Sources/Blockchain/Validator/GuaranteeingService.swift:523, 530
Gas limit checks use strict inequality (<), which disallows usage exactly equal to the limit. Protocol limits are typically inclusive (<=). For example, totalRefineGas < config.value.workPackageRefineGas should likely be <=. Note that the dependency check on line 538 uses <=.
Refactor the accumulation statistics logic to use pre-calculated dictionaries for digest counts and gas usage, eliminating repeated filtering inside the loop.
Update the `reed_solomon_recovery` FFI to accept both original and parity shards, allowing the underlying Rust library to utilize available original data during reconstruction. The Swift wrapper now separates input shards based on index and prefills the output buffer with known original data.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #376 +/- ##
==========================================
- Coverage 81.63% 81.09% -0.54%
==========================================
Files 380 359 -21
Lines 26006 25135 -871
==========================================
- Hits 21229 20384 -845
+ Misses 4777 4751 -26 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Uh oh!
There was an error while loading. Please reload this page.