Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix: write failed evm receipts after execution, rather than at the end of block #2525

Merged
merged 3 commits into from
Jun 12, 2023
Merged

fix: write failed evm receipts after execution, rather than at the end of block #2525

merged 3 commits into from
Jun 12, 2023

Conversation

jorgemmsilva
Copy link
Contributor

No description provided.

dessaya
dessaya reviewed Jun 6, 2023
View reviewed changes
packages/vm/core/evm/emulator/emulator.go Outdated Show resolved Hide resolved
packages/vm/core/evm/emulator/emulator.go Outdated Show resolved Hide resolved
@jorgemmsilva
Copy link
Contributor Author

@dessaya pushed an update where the ISC dependency is removed from evm/emulator.

While I understand the wish to keep both layers separated, they are already connected in the sense that we pass functions like gasBurnEnable which is ISC-only functionality. I guess not importing the package directly does reduce the coupling in a way, but the logic is intertwined, extracting into generic fuctions just adds a bit more navigational effort IMO.

@dessaya
Copy link
Collaborator

dessaya commented Jun 12, 2023

Yes, I agree. But still, I think it's beneficial to keep this separation, as it makes the emulator easier to test in isolation.

dessaya
dessaya approved these changes Jun 12, 2023
View reviewed changes
packages/vm/core/evm/emulator/emulator.go
Comment on lines +291 to +296
// chargeISCGas will keep gasBurnEnabled = false and mutate `result` when charging ISC gas fails
if chargeISCGas != nil {
gasUsed, iscGasErr = chargeISCGas(result)
} else {
gasUsed = result.UsedGas
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

wasp/packages/vm/core/evm/evmimpl/iscmagic.go

Line 110 in a302676

return ret, gas, executionErr

Maybe it's possible to obtain the same result by modifying the logic there so that it deducts the gas for each ISC call automatically.

But we can do that in a separate PR.

@jorgemmsilva jorgemmsilva merged commit 742a147 into iotaledger:develop Jun 12, 2023
@jorgemmsilva jorgemmsilva mentioned this pull request Jun 13, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dessaya dessaya dessaya approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jorgemmsilva @dessaya