Skip to content

Conversation

@m-Peter
Copy link
Collaborator

@m-Peter m-Peter commented Mar 22, 2025

Work towards: #7152
Depends on: onflow/go-ethereum#12

@m-Peter m-Peter self-assigned this Mar 22, 2025
@codecov-commenter
Copy link

codecov-commenter commented Mar 22, 2025

Codecov Report

Attention: Patch coverage is 44.18605% with 24 lines in your changes missing coverage. Please review.

Project coverage is 41.30%. Comparing base (6e6c6cd) to head (e6846a9).

Files with missing lines Patch % Lines
fvm/evm/emulator/state/stateDB.go 55.17% 13 Missing ⚠️
fvm/evm/types/call.go 0.00% 5 Missing ⚠️
fvm/evm/emulator/emulator.go 0.00% 4 Missing ⚠️
fvm/evm/emulator/state/delta.go 60.00% 2 Missing ⚠️
Additional details and impacted files
@@                    Coverage Diff                     @@
##           feature/pectra-upgrade    #7175      +/-   ##
==========================================================
- Coverage                   41.33%   41.30%   -0.03%     
==========================================================
  Files                        2164     2164              
  Lines                      189596   189623      +27     
==========================================================
- Hits                        78370    78333      -37     
- Misses                     104705   104761      +56     
- Partials                     6521     6529       +8     
Flag Coverage Δ
unittests 41.30% <44.18%> (-0.03%) ⬇️

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

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

@m-Peter m-Peter force-pushed the mpeter/update-go-ethereum-v1.14.12 branch from ab2fcde to e6846a9 Compare March 24, 2025 07:44
@m-Peter m-Peter marked this pull request as ready for review March 24, 2025 08:00
@m-Peter m-Peter changed the base branch from feature/pectra-upgrade to mpeter/update-go-ethereum-v1.14.11 April 3, 2025 11:53

// SetState adds sets a value for the given slot of the main storage
func (d *DeltaView) SetState(sk types.SlotAddress, value gethCommon.Hash) error {
// SetState adds sets a value for the given slot of the main storage.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// SetState adds sets a value for the given slot of the main storage.
// SetState sets or adds a value for the given slot of the main storage.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated in 53c44ce .

Comment on lines +122 to +123
balance, err := db.latestView().GetBalance(addr)
db.handleError(err)
Copy link
Contributor

Choose a reason for hiding this comment

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

should this happen before SelfDestruct?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's a great catch 💯 Updated in 05b8ee2

}

func (db *StateDB) Finalise(deleteEmptyObjects bool) {
panic(fmt.Errorf("should not be called!"))
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if it would be better to call Finalise but leave it empty. Might be more in line with how its envisioned.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point indeed. Updated in a69d18c .

@m-Peter
Copy link
Collaborator Author

m-Peter commented Apr 9, 2025

Superseded by #7241 .

@m-Peter m-Peter closed this Apr 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants