Modularized storage capturing in OpcodeTracer #10307
Conversation
Signed-off-by: Bilyana Gospodinova <[email protected]>
Signed-off-by: Bilyana Gospodinova <[email protected]>
Signed-off-by: Bilyana Gospodinova <[email protected]>
Signed-off-by: Bilyana Gospodinova <[email protected]>
Signed-off-by: Bilyana Gospodinova <[email protected]>
Signed-off-by: Kristiyan Selveliev <[email protected]>
kselveliev
added
modularizedEVM
web3
| @@ -57,11 +66,20 @@ | |||
| @Getter | |||
| public class OpcodeTracer implements HederaOperationTracer { | |||
|
|
|||
| private static final String CONTRACT_SERVICE = "ContractService"; | |||
There was a problem hiding this comment.
How about ContractService.NAME?
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #10307 +/- ##
============================================
+ Coverage 92.17% 92.24% +0.07%
- Complexity 7966 8072 +106
============================================
Files 975 984 +9
Lines 33278 33643 +365
Branches 4201 4250 +49
============================================
+ Hits 30673 31033 +360
- Misses 1606 1608 +2
- Partials 999 1002 +3 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Bilyana Gospodinova <[email protected]>
Signed-off-by: Kristiyan Selveliev <[email protected]>
…modularized Signed-off-by: Kristiyan Selveliev <[email protected]>
Signed-off-by: Kristiyan Selveliev <[email protected]>
Signed-off-by: Kristiyan Selveliev <[email protected]>
| @@ -101,6 +114,8 @@ | |||
| @ExtendWith(MockitoExtension.class) | |||
| class OpcodeTracerTest { | |||
|
|
|||
| private static final String CONTRACT_SERVICE = "ContractService"; | |||
There was a problem hiding this comment.
Duplicated constants as in OpcodeTracer.
| Set<SlotKey> modifiedKeys = states.get(STORAGE_KEY).modifiedKeys().stream() | ||
| .filter(SlotKey.class::isInstance) | ||
| .map(SlotKey.class::cast) | ||
| .filter(slotKey -> slotKey.hasContractID() && slotKey.contractID() != null) |
There was a problem hiding this comment.
Isn't that the same check? slotKey.hasContractID() internally checks if the contract ID is not null.
There was a problem hiding this comment.
It's the same. I used it to remove ide warnings below but seems not applicable with latest changes. I removed it now.
| @@ -246,4 +269,58 @@ private Bytes formatRevertReason(final Bytes revertReason) { | |||
|
|
|||
| return BytesDecoder.getAbiEncodedRevertReason(revertReason); | |||
| } | |||
|
|
|||
| private Map<Bytes, Bytes> getStorageUpdates(Address accountAddress) { | |||
There was a problem hiding this comment.
Maybe the return type could be wrapped in an Optional and in the caller it can be chained to avoid some empty checks.
| Bytes.of(slotValue.value().toByteArray())); | ||
| } | ||
| } | ||
| } catch (IllegalArgumentException e) { |
There was a problem hiding this comment.
Could this exception actually occur? This is a private method and will be called only within the class.
There was a problem hiding this comment.
This can happen when we try to retrieve the storage state ->
states.get(STORAGE_KEY).modifiedKeys() ->
if (state == null) {
throw new IllegalArgumentException("Unknown k/v state key: " + stateKey);
}
if (!(state instanceof WritableKVState)) {
throw new IllegalArgumentException("State is not an instance of WritableKVState: " + stateKey);
}
Signed-off-by: Kristiyan Selveliev <[email protected]>
Signed-off-by: Kristiyan Selveliev <[email protected]>
Signed-off-by: Kristiyan Selveliev <[email protected]>
|
|
Closing in favour of https://github.com/hashgraph/hedera-mirror-node/pull/10357 |
…0357) This PR is a copy of #10307. (It was checkout from the pr fixing the states caching but it will be delayed due to needed upstream changes). So opening with main as base branch. This PR fixes storage capturing in modularized execution. Storage values are now read from mirror node writable states during execution. Added a test who actually modifies storage of a contract and can verify execution. This pr also fixes 4 failing test in OpcodeServiceTests. Now OpcodeServiceTest fixes are completed. Changes: OpcodeTracer - reads storage changes from writablestates - STORAGE_KEY for modularized OpcodeTracerTest - adds codecov for new methods OpcodeServiceTest - adds new test contract which modifies contract storage and verifies storage changes --------- Signed-off-by: Kristiyan Selveliev <[email protected]>
Description:
This PR fixes storage capturing in modularized execution.
Storage values are now read from mirror node writable states during execution.
Added a test who actually modifies storage of a contract and can verify execution.
This pr also fixes 4 failing test in
OpcodeServiceTests. NowOpcodeServiceTestfixes are completed.This PR is checkout from -> https://github.com/hashgraph/hedera-mirror-node/pull/10306
It only works with the fixes for the state pollution must be merged after the above PR.
Only relevant changes from the PR - https://github.com/hashgraph/hedera-mirror-node/pull/10307/commits/33eea994a5bfce0a46895b50070307d87d911cd0
Related issue(s):
Fixes #10259
Notes for reviewer:
Checklist