Skip to content

Conversation

@qiweiii
Copy link
Contributor

@qiweiii qiweiii commented Sep 23, 2025

No description provided.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

The changes introduce a new copying mechanism for state, which is a good improvement. However, there are several issues with the implementation. A key issue is the incorrect handling of mutable state in the new copy initializer for ServiceAccountsMutRef, which leads to shared mutable state instead of a true copy. Additionally, there's a potentially incorrect clearing of state changes in Accumulation.swift and some inefficient lazy initialization patterns have been introduced in HostCalls.swift.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Found a mutability/shadowing bug when building a dictionary in Bless, and an unsafe force-cast plus aliasing of mutable state in ServiceAccounts copy initializer.

@qiweiii qiweiii changed the title fix to pass traces fuzz v1 and fix Sep 24, 2025
@github-actions
Copy link

No issues found

@codecov
Copy link

codecov bot commented Sep 24, 2025

Codecov Report

❌ Patch coverage is 27.44186% with 156 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.74%. Comparing base (0a75453) to head (10e3011).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
Fuzzing/Sources/Fuzzing/FuzzingTarget.swift 0.00% 89 Missing ⚠️
Fuzzing/Sources/Fuzzing/FuzzingClient.swift 0.00% 36 Missing ⚠️
Fuzzing/Sources/Fuzzing/Messages.swift 0.00% 20 Missing ⚠️
...ces/Blockchain/RuntimeProtocols/Guaranteeing.swift 27.27% 8 Missing ⚠️
.../Blockchain/VMInvocations/HostCall/HostCalls.swift 76.92% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #368      +/-   ##
==========================================
- Coverage   81.96%   81.74%   -0.22%     
==========================================
  Files         381      381              
  Lines       33758    33912     +154     
==========================================
+ Hits        27670    27723      +53     
- Misses       6088     6189     +101     

☔ 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.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Found multiple correctness and robustness issues in the changed code, including a likely compile error in HostCalls optional binding and logic regression, inconsistent error handling between target and client, a forced unwrap that can crash, and a potentially incorrect use of try on a non-throwing initializer.

@qiweiii qiweiii marked this pull request as ready for review September 24, 2025 02:45
@qiweiii qiweiii requested a review from xlc September 24, 2025 04:23
@xlc xlc merged commit 07b8b47 into master Sep 25, 2025
5 of 7 checks passed
@xlc xlc deleted the fix-v070 branch September 25, 2025 02:33
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