Skip to content

Conversation

@matkt
Copy link
Contributor

@matkt matkt commented Feb 16, 2023

PR description

Fixed Issue(s)

Documentation

  • I thought about documentation and added the doc-change-required label to this PR if
    updates are required.

Acceptance Tests (Non Mainnet)

  • I have considered running ./gradlew acceptanceTestNonMainnet locally if my PR affects non-mainnet modules.

Changelog

if (localCode == null) {
return wrappedWorldView().getCode(address, codeHash);
final Optional<Bytes> code = wrappedWorldView().getCode(address, codeHash);
if (code.isEmpty() && !codeHash.equals(Hash.EMPTY)) {
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 instead hash code and compare it to codeHash rather than just checking empty? or is the hashing prohibitively expensive

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes this is enough because the hash check is already done at the storage level. And if it is not good we will have an Empty. So I prefer to avoid adding an unnecessary extra shot for this check

@matkt matkt marked this pull request as ready for review February 17, 2023 14:41
Copy link
Contributor

@garyschulte garyschulte left a comment

Choose a reason for hiding this comment

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

🚢

@matkt matkt enabled auto-merge (squash) February 20, 2023 10:58
@matkt matkt merged commit 17455d6 into hyperledger:main Feb 20, 2023
ensi321 pushed a commit to ensi321/besu that referenced this pull request Mar 6, 2023
elenduuche pushed a commit to elenduuche/besu that referenced this pull request Aug 16, 2023
@matkt matkt deleted the feature/fix-code branch September 22, 2023 09:41
eum602 pushed a commit to lacchain/besu that referenced this pull request Nov 3, 2023
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.

2 participants