-
Notifications
You must be signed in to change notification settings - Fork 138
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
03640 Contract create nonce externalization #6934
Conversation
Signed-off-by: Alexander Gadzhalov <[email protected]>
Signed-off-by: Alexander Gadzhalov <[email protected]>
…tion Signed-off-by: Alexander Gadzhalov <[email protected]>
Signed-off-by: Alexander Gadzhalov <[email protected]>
Signed-off-by: Alexander Gadzhalov <[email protected]>
…Record method Signed-off-by: Alexander Gadzhalov <[email protected]>
Signed-off-by: Alexander Gadzhalov <[email protected]>
Signed-off-by: Alexander Gadzhalov <[email protected]>
...-clients/src/main/java/com/hedera/services/bdd/suites/contract/hapi/ContractCreateSuite.java
Outdated
Show resolved
Hide resolved
…Unit test minor changes Signed-off-by: Alexander Gadzhalov <[email protected]>
Signed-off-by: Alexander Gadzhalov <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, tyvm @agadzhalov 🥂
@agadzhalov appreciate the PR description. Nice job 👍🏾 |
The embedded links to the source is an especially nice touch. Should be a model for any large&nuanced PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is as far as I've gotten. Will return to it shortly. No problems so far but a bunch of questions.
...service/src/main/java/com/hedera/node/app/service/mono/store/contracts/HederaWorldState.java
Show resolved
Hide resolved
...service/src/main/java/com/hedera/node/app/service/mono/store/contracts/HederaWorldState.java
Show resolved
Hide resolved
...service/src/main/java/com/hedera/node/app/service/mono/store/contracts/HederaWorldState.java
Show resolved
Hide resolved
...service/src/main/java/com/hedera/node/app/service/mono/store/contracts/HederaWorldState.java
Show resolved
Hide resolved
...service/src/main/java/com/hedera/node/app/service/mono/store/contracts/HederaWorldState.java
Show resolved
Hide resolved
...evm/src/main/java/com/hedera/node/app/service/evm/store/contracts/HederaEvmEntityAccess.java
Show resolved
Hide resolved
...ra-evm/src/main/java/com/hedera/node/app/service/evm/store/models/UpdateTrackingAccount.java
Show resolved
Hide resolved
...ra-evm/src/main/java/com/hedera/node/app/service/evm/store/models/UpdateTrackingAccount.java
Show resolved
Hide resolved
...ra-evm/src/main/java/com/hedera/node/app/service/evm/store/models/UpdateTrackingAccount.java
Show resolved
Hide resolved
...ra-evm/src/main/java/com/hedera/node/app/service/evm/store/models/UpdateTrackingAccount.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM. Lots of comments and suggestions - if you're going to make changes for these suggestions/comments then they don't have to be in this PR - but if you do defer them to a latter PR don't forget about it ...
...o-service/src/main/java/com/hedera/node/app/service/mono/store/UpdateAccountTrackerImpl.java
Show resolved
Hide resolved
...service/src/main/java/com/hedera/node/app/service/mono/store/contracts/HederaWorldState.java
Show resolved
Hide resolved
...ice/src/test/java/com/hedera/node/app/service/mono/records/TxnAwareRecordsHistorianTest.java
Show resolved
Hide resolved
hedera-node/hedera-mono-service/src/test/java/com/hedera/test/utils/SeededPropertySource.java
Show resolved
Hide resolved
...t/java/com/hedera/node/app/service/mono/txns/contract/ContractCreateTransitionLogicTest.java
Show resolved
Hide resolved
...t/java/com/hedera/node/app/service/mono/txns/contract/ContractCreateTransitionLogicTest.java
Show resolved
Hide resolved
Unit test coverage for new code looks pretty good. |
52ae50a
6a9eea9
to
c0294f6
Compare
# Conflicts: # hedera-node/hedera-mono-service/src/test/java/com/hedera/node/app/service/mono/state/submerkle/EvmFnResultSerdeTest.java # hedera-node/hedera-mono-service/src/test/java/com/hedera/test/serde/SerializedForms.java # hedera-node/hedera-mono-service/src/test/java/com/hedera/test/utils/SeededPropertySource.java
Refactor and enhancement will be done here #7376 |
Signed-off-by: Alexander Gadzhalov <[email protected]>
bfb6150
to
7bb8b24
Compare
Kudos, SonarCloud Quality Gate passed! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Description:
Externalizes and verifies the nonces on contract create.
Implementation of contracts nonces externalization
HederaWorldState
which is called only on enabledisContractsNoncesExternalizationEnabled
feature flag.1.1 Checks if an account is a new smart contract and externalizes its nonce.
1.2 Checks if an existing smart contract's nonce is updated and externalizes it.
contractId
andnonce
EvmFnResult
that buildsList<ContractNonceInfo>
(submerkle) fromMap<ContractID, Long>
ContractNonceInfo
(submerkle) to grpc (protobuf)7
(RELEASE_0400_VERSION
) and externalized logic for serialize and deserialize ofcontractNonces
inEvmFnResult
E2E testing
ContractFnResultAsserts
in order to assert the expected and actual nonce of the newly created contract.contractCreateNoncesExternalizationHappyPath
test to verify that the nonces of the created contract are incremented and externalized properly.contractCreateFollowedByContractCallNoncesExternalization
contract_nonces
entries were created and they are null.shouldReturnNullWhenContractsNoncesExternalizationFlagIsDisabled
test to verify that there will be nocontract_nonces
entries when the feature flag is disabled.Related issue(s):
Fixes #3640