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.

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

Bonsai Tries: Respect account deletion across transacitons #1952

Merged
merged 7 commits into from
Feb 26, 2021

Conversation

shemnon
Copy link
Contributor

@shemnon shemnon commented Feb 25, 2021

PR description

In a block if another account is deleted vs self destruct the sstore
operation could get an incorrect original value, and if the new write is
the same as the value before delete a SLOAD cost will mistakenly be
applied. Found in mainnet block 11905274 for account
0x7f150bd6f54c40a34d7c3d5e9f56.

Fixed Issue(s)

Changelog

The updated side of a bonsaiValue may legitalately be null,
indicatinga  dalete.  In those cases optionals should contain empty.

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
In a block if another account is deleted vs self destruct the sstore
operation could get an incorrect original value, and if the new write is
the same as the value before delete a SLOAD cost will mistakenly be
applied.  Found in mainnet block 11905274 for account
0x7f150bd6f54c40a34d7c3d5e9f56.

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon shemnon marked this pull request as ready for review February 25, 2021 19:49
@atoulme
Copy link
Contributor

atoulme commented Feb 26, 2021

Looks good to me, would there be a way to have a test to back it up?

Copy link
Contributor

@matkt matkt left a comment

Choose a reason for hiding this comment

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

Sounds good to me.

@shemnon shemnon merged commit b23370c into hyperledger:master Feb 26, 2021
shemnon added a commit that referenced this pull request Feb 26, 2021
When in bonsai mode, if inside one block an account is deleted via self
destruct and then the account is re-created the latter sstore operation
reports the last SSTORE value prior to self destruct. Thus if the new
write is the same as the value before delete a non-updating gas cost
will mistakenly be applied.  Thus we first check accountWasDeleted when
reading original slot values.

Found in mainnet block 11905274 for account
0x7f150bd6f54c40a34d7c3d5e9f56.

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
RichardH92 pushed a commit to RichardH92/besu that referenced this pull request Mar 29, 2021
…er#1952)

When in bonsai mode, if inside one block an account is deleted via self
destruct and then the account is re-created the latter sstore operation
reports the last SSTORE value prior to self destruct. Thus if the new
write is the same as the value before delete a non-updating gas cost
will mistakenly be applied.  Thus we first check accountWasDeleted when
reading original slot values.

Found in mainnet block 11905274 for account
0x7f150bd6f54c40a34d7c3d5e9f56.

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
Signed-off-by: Richard Hart <richardhart92@gmail.com>
@shemnon shemnon deleted the bonsaiNull branch February 26, 2022 18:44
eum602 pushed a commit to lacchain/besu that referenced this pull request Nov 3, 2023
…er#1952)

When in bonsai mode, if inside one block an account is deleted via self
destruct and then the account is re-created the latter sstore operation
reports the last SSTORE value prior to self destruct. Thus if the new
write is the same as the value before delete a non-updating gas cost
will mistakenly be applied.  Thus we first check accountWasDeleted when
reading original slot values.

Found in mainnet block 11905274 for account
0x7f150bd6f54c40a34d7c3d5e9f56.

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
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.

3 participants