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

Deep VM Queries (for older epochs, as well) #5801

Merged
merged 22 commits into from Feb 15, 2024

Conversation

andreibancioiu
Copy link
Collaborator

@andreibancioiu andreibancioiu commented Jan 4, 2024

Reasoning behind the pull request

Enabled VM queries for older (all) epochs. Previously, VM queries were possible at blocks within epochs covered by the active persisters (e.g. last 3 epochs).

Pre-requisites

Based on the Contributing Guidelines the PR author and the reviewers must check the following requirements are met:

  • was the PR targeted to the correct branch?
  • if this is a larger feature that probably needs more than one PR, is there a feat branch created?
  • if this is a feat branch merging, do all satellite projects have a proper tag inside go.mod?

@andreibancioiu andreibancioiu self-assigned this Jan 4, 2024
Copy link

codecov bot commented Feb 6, 2024

Codecov Report

Attention: 9 lines in your changes are missing coverage. Please review.

Comparison is base (5531e7f) 80.21% compared to head (624413b) 80.17%.

Files Patch % Lines
factory/api/apiResolverFactory.go 70.00% 7 Missing and 2 partials ⚠️
Additional details and impacted files
@@                Coverage Diff                @@
##           rc/v1.6.next1    #5801      +/-   ##
=================================================
- Coverage          80.21%   80.17%   -0.04%     
=================================================
  Files                708      708              
  Lines              94124    94129       +5     
=================================================
- Hits               75497    75466      -31     
- Misses             13282    13316      +34     
- Partials            5345     5347       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@andreibancioiu andreibancioiu changed the title [Do not review] Sandbox for vm queries. Deep VM Queries (for older epochs, as well) Feb 9, 2024
Add integration test for deep queries
@andreibancioiu
Copy link
Collaborator Author

☑️ Tested using integration test: #5935

☑️ Tested on Testnet:

  1. sent transactions that alter the state of the smart contract, across multiple epochs
  2. sent transactions the reveal the state root hash / block nonce etc. (at the moment of transaction processing)
  3. performed deep queries to check the state at various blocks in the past - with respect to point (1)
  4. performed deep queries to query the perceived "now" (state root hash, block nonce etc.) for blocks that held transactions from point (2)

Contract used to test on Testnet:
https://testnet-explorer.multiversx.com/accounts/erd1qqqqqqqqqqqqqpgq6xcct3kqdcsqndw2mmv29fqh9zq6yyp5396qszts9z

@iulianpascalau iulianpascalau changed the base branch from master to rc/v1.6.next1 February 9, 2024 16:46
@iulianpascalau iulianpascalau marked this pull request as ready for review February 9, 2024 16:46

require.Equal(t, snapshotsOfGetState[1], historyOfGetState[1])
require.Equal(t, snapshotsOfGetNow[1].blockNonce, historyOfGetNow[1].blockNonce)
// This does not seem right!
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Though, I think the issue is with the test node, not with the actual implementation of deep queries. Actual implementation tested on system-test / Testnet ☑️

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed that (redundant & ambiguous) assertion.

sstanculeanu
sstanculeanu previously approved these changes Feb 12, 2024
@@ -356,10 +356,20 @@ func createScQueryElement(
return nil, errDecode
}

apiBlockchain, err := blockchain.NewBlockChain(disabled.NewAppStatusHandler())
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Here we should handle metachain, as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in #5958.

sstanculeanu
sstanculeanu previously approved these changes Feb 14, 2024
@@ -47,3 +48,7 @@ func CreateScQueryElement(args SCQueryElementArgs) (process.SCQueryService, erro
guardedAccountHandler: args.GuardedAccountHandler,
})
}

func CreateBlockchainForScQuery(selfShardID uint32) (data.ChainHandler, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

missing comment

Copy link
Contributor

Choose a reason for hiding this comment

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

fixed

gabi-vuls
gabi-vuls previously approved these changes Feb 14, 2024
Copy link
Collaborator

@gabi-vuls gabi-vuls left a comment

Choose a reason for hiding this comment

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

Normal allin test: v1.6.15-dev-config-eb2e06c06d -> sandbox-queries-dc797c7a20

--- Specific errors ---

block hash does not match 1448
wrong nonce in block 781
miniblocks does not match 0
num miniblocks does not match 0
miniblock hash does not match 0
block bodies does not match 0
receipts hash missmatch 0

/------/

--- Statistics ---

Nr. of all ERRORS: 4
Nr. of all WARNS: 573
Nr. of new ERRORS: 0
Nr. of new WARNS: 4
Nr. of PANICS: 0

/------/

--- ERRORS ---

/------/

--- WARNINGS ---

upcloud-fra-validator-19 :
Warn: shardProcessor.createMiniBlocks error = shard is stuck shard = 2

upcloud-fra-validator-29 :
Warn: shardProcessor.createMiniBlocks error = shard is stuck shard = 2

/------/

Copy link
Collaborator

@gabi-vuls gabi-vuls left a comment

Choose a reason for hiding this comment

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

Normal allin test: v1.6.15-dev-config-eb2e06c06d -> sandbox-queries-dc797c7a20

--- Specific errors ---

block hash does not match 1448
wrong nonce in block 781
miniblocks does not match 0
num miniblocks does not match 0
miniblock hash does not match 0
block bodies does not match 0
receipts hash missmatch 0

/------/

--- Statistics ---

Nr. of all ERRORS: 4
Nr. of all WARNS: 573
Nr. of new ERRORS: 0
Nr. of new WARNS: 4
Nr. of PANICS: 0

/------/

--- ERRORS ---

/------/

--- WARNINGS ---

upcloud-fra-validator-19 :
Warn: shardProcessor.createMiniBlocks error = shard is stuck shard = 2

upcloud-fra-validator-29 :
Warn: shardProcessor.createMiniBlocks error = shard is stuck shard = 2

/------/

@iulianpascalau iulianpascalau merged commit 6faf35a into rc/v1.6.next1 Feb 15, 2024
6 of 8 checks passed
@iulianpascalau iulianpascalau deleted the sandbox-queries branch February 15, 2024 16:13
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.

None yet

5 participants