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

Get rid of transaction receipt root #3188

Closed
itegulov opened this issue Oct 29, 2024 · 0 comments · Fixed by #3187
Closed

Get rid of transaction receipt root #3188

itegulov opened this issue Oct 29, 2024 · 0 comments · Fixed by #3187
Assignees
@itegulov

Comments

@itegulov
Copy link
Contributor

We include both status and root fields in eth_getTransactionReceipt and eth_getBlockReceipts which is incorrect behaviour. EIP98 set root to 0x0/0x1 and then EIP658 further renamed root to status. We never computed root correctly anyway as we just set it to the same value as block_hash, unsure if there was ever any purpose in that (git history shows that this logic was present from the very first commit).

Some ETH tooling gets confused by this as both fields are never supposed to be present together. For example alloy rejects such receipts as having duplicate status.

I propose we drop root altogether to remove ambiguity.
@itegulov itegulov mentioned this issue Oct 29, 2024
Merged
4 tasks
github-merge-queue bot pushed a commit that referenced this issue Oct 29, 2024
@itegulov
feat(api): get rid of tx receipt root (#3187)
6c034f6 
## What ❔

Transaction receipt `root` was replaced by `status` in
[EIP658](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-658.md).
Since we already return `status` there is no need in having `root` too.

Fixes #3188 

## Why ❔

It is actively harmful as it confuses some ETH tooling that considers
having both `status` and `root` at the same time an invalid receipt.

## Checklist

<!-- Check your PR fulfills the following items. -->
<!-- For draft PRs check the boxes as you complete them. -->

- [ ] PR title corresponds to the body of PR (we generate changelog
entries from PRs).
- [ ] Tests for the changes have been added / updated.
- [ ] Documentation comments have been added / updated.
- [ ] Code has been formatted via `zkstack dev fmt` and `zkstack dev
lint`.
@itegulov itegulov self-assigned this Nov 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@itegulov itegulov

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(api): get rid of tx receipt root matter-labs/zksync-era
1 participant
@itegulov