Skip to content

Conversation

@qiweiii
Copy link
Contributor

@qiweiii qiweiii commented Nov 20, 2025

one thing to note, I separated config limited array's decode and validate, since fuzzer expect targets to be able to decode messages with such invalid blocks, which might be acceptable since gp did not say codec need to do block validation, and I made sure our Validate goes to all child types of block, so when import block, we definitely validate all those

@github-actions
Copy link

Validation checks were removed from the limited-size array decoders, which now lets invalid lengths slip through for several non-Validate element types.


Review Suggestions

Utils/Sources/Utils/ConfigLimitedSizeArray.swift:43-57, 236-248

The decoding initializer now constructs arrays with validate: false and never re-invokes validateThrowing(). For many uses the element type does not conform to Validate (e.g., ConfigLimitedSizeArray<Data32,…> in the authorization pool), so there is no subsequent validation step to enforce minLength/maxLength. The change therefore allows untrusted input to produce arrays of arbitrary length, breaking the invariants this type is supposed to guarantee and potentially letting oversized pools or empty arrays into runtime logic. Please keep calling validateThrowing() at the end of init(from:) (or only skip validation when you have a guaranteed later validation pass), so the size constraints remain enforced for every decoded value.


Utils/Sources/Utils/LimitedSizeArray.swift:22-29, 154-176

Similarly, LimitedSizeArray’s public initializer now accepts a validate flag and the Decodable initializer always passes validate: false without re-running validateThrowing(). Because this type only conforms to Validate when T: Validate, arrays of primitive types such as LimitedSizeArray<TimeslotIndex, ConstInt0, ConstInt3> (used in preimage info state) now never enforce their compile-time bounds during decoding. That means any stored value can decode with too many or too few elements, violating the assumptions of callers and inviting logic errors. Reintroduce a try validateThrowing() at the end of the decoder (or otherwise ensure validation still happens), so decoding continues to uphold the min/max length guarantees regardless of the element type.

@github-actions
Copy link

No issues found. The changes correctly separate decoding and validation for limited size arrays, enabling fuzzing of invalid blocks. The Validate protocol updates ensure that validation is still performed when required. The deterministic sorting in Accumulation.swift is a good addition.

@codecov
Copy link

codecov bot commented Nov 20, 2025

Codecov Report

❌ Patch coverage is 88.88889% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.70%. Comparing base (0645b4d) to head (817baea).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
...Sources/Blockchain/RuntimeProtocols/Validate.swift 25.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #375      +/-   ##
==========================================
- Coverage   81.74%   81.70%   -0.05%     
==========================================
  Files         380      380              
  Lines       25997    26006       +9     
==========================================
- Hits        21252    21248       -4     
- Misses       4745     4758      +13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@qiweiii qiweiii marked this pull request as ready for review November 20, 2025 01:35
@qiweiii qiweiii requested a review from xlc November 20, 2025 02:30
@qiweiii qiweiii merged commit 77d4fe6 into master Nov 20, 2025
9 of 11 checks passed
@qiweiii qiweiii deleted the traces-round-0.7.1 branch November 20, 2025 02:48
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.

3 participants