Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: add to AccountProof to trace #98

Merged
merged 9 commits into from
May 17, 2022
Merged

feat: add to AccountProof to trace #98

merged 9 commits into from
May 17, 2022

Conversation

0xmountaintop
Copy link

@0xmountaintop 0xmountaintop commented May 16, 2022

Turns out that we forget to add to AccountProof in the trace.
https://github.com/appliedzkp/zkevm-circuits/blob/287a3f4dfe38d13a7ab920c2d781fb26fd519179/bus-mapping/src/circuit_input_builder/access.rs#L124-L127

This wasn't detected because previously we use created smart contract address as the receiver:
https://github.com/scroll-tech/integration-test/pull/25

@ChuhanJin
Copy link

Can one of the admins verify this patch?

@0xmountaintop 0xmountaintop requested a review from mask-pp May 16, 2022 10:34
@0xmountaintop 0xmountaintop requested a review from jules May 17, 2022 02:02
miner/worker.go Outdated
@@ -794,6 +794,17 @@ func (w *worker) commitTransaction(tx *types.Transaction, coinbase common.Addres
Balance: (*hexutil.Big)(w.current.state.GetBalance(from)),
CodeHash: w.current.state.GetCodeHash(from),
}
// Get receiver's address.
receiver := (*types.AccountProofWrapper)(nil)
Copy link

Choose a reason for hiding this comment

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

Use var receiver *types.AccountProofWrapper maybe much better.

mask-pp
mask-pp previously approved these changes May 17, 2022
@scroll-dev scroll-dev merged commit 571dcad into zkrollup May 17, 2022
@scroll-dev scroll-dev deleted the fix/to branch May 17, 2022 21:43
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants