Skip to content

fix(devnet-sdk): fix colliding sentinels#14512

Closed
sigma wants to merge 1 commit intodevelopfrom
sigma/fix-sentinels
Closed

fix(devnet-sdk): fix colliding sentinels#14512
sigma wants to merge 1 commit intodevelopfrom
sigma/fix-sentinels

Conversation

@sigma
Copy link
Contributor

@sigma sigma commented Feb 24, 2025

Description

The code was wrongly assuming that the compiler didn't optimize out
separate pointers pointing to an identical object.

Tests

Additional context

Metadata

@sigma sigma requested a review from a team as a code owner February 24, 2025 23:33
@sigma sigma requested a review from sebastianst February 24, 2025 23:33
@codecov
Copy link

codecov bot commented Feb 24, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 41.97%. Comparing base (b1ff39c) to head (ef4de73).
Report is 77 commits behind head on develop.

❗ There is a different number of reports uploaded between BASE (b1ff39c) and HEAD (ef4de73). Click for more details.

HEAD has 4 uploads less than BASE
Flag BASE (b1ff39c) HEAD (ef4de73)
2 1
cannon-go-tests-32 1 0
cannon-go-tests-64 1 0
contracts-bedrock-tests 1 0
Additional details and impacted files
@@             Coverage Diff              @@
##           develop   #14512       +/-   ##
============================================
- Coverage    77.23%   41.97%   -35.27%     
============================================
  Files          178      859      +681     
  Lines        10649    78476    +67827     
============================================
+ Hits          8225    32939    +24714     
- Misses        2243    42644    +40401     
- Partials       181     2893     +2712     
Flag Coverage Δ
cannon-go-tests-32 ?
cannon-go-tests-64 ?
contracts-bedrock-tests ?

Flags with carried forward coverage won't be shown. Click here to find out more.

see 1029 files with indirect coverage changes


func AcquireLowLevelSystem() (LowLevelSystemGetter, systest.PreconditionValidator) {
sysMarker := &struct{}{}
sysMarker := newSentinelMarker()
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you should just be able to use new(byte) instead of &struct{}{} instead of keeping a global counter or?

I made that fix and validated it by a run in the external devnet-sdk testing repo, see description in the PR #14514

@sigma sigma closed this Mar 5, 2025
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.

2 participants

Comments