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

Tx simulator fix #3530

Merged
merged 7 commits into from
Oct 22, 2021
Merged

Tx simulator fix #3530

merged 7 commits into from
Oct 22, 2021

Conversation

miiu96
Copy link
Contributor

@miiu96 miiu96 commented Oct 22, 2021

Bug-fixes on transaction simulator.

@codecov
Copy link

codecov bot commented Oct 22, 2021

Codecov Report

Merging #3530 (e81af38) into master (dc76fa4) will decrease coverage by 0.01%.
The diff coverage is 56.09%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3530      +/-   ##
==========================================
- Coverage   73.88%   73.87%   -0.02%     
==========================================
  Files         582      582              
  Lines       74585    74618      +33     
==========================================
+ Hits        55108    55122      +14     
- Misses      15073    15086      +13     
- Partials     4404     4410       +6     
Impacted Files Coverage Δ
process/txsimulator/wrappedAccountsDB.go 78.26% <0.00%> (-3.56%) ⬇️
state/accountsDB.go 76.91% <33.33%> (-0.43%) ⬇️
process/block/metablock.go 55.09% <50.00%> (-0.01%) ⬇️
factory/blockProcessorCreator.go 79.21% <55.61%> (-1.17%) ⬇️
factory/processComponents.go 62.58% <100.00%> (+0.12%) ⬆️
process/block/shardblock.go 66.74% <100.00%> (+0.08%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update df366fb...e81af38. Read the comment docs.

@miiu96 miiu96 marked this pull request as ready for review October 22, 2021 12:58
@@ -189,6 +189,20 @@
MaxBatchSize = 100
MaxOpenFiles = 10

[SmartContractsStorageSimulate]
Copy link
Contributor

Choose a reason for hiding this comment

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

SmartContractsStorageMock or SmartContractsStorageForTxSimulator instead SmartContractsStorageSimulate to each name here ?

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

@@ -124,6 +124,7 @@ type Config struct {
SmartContractsStorage StorageConfig
SmartContractsStorageForSCQuery StorageConfig
TrieEpochRootHashStorage StorageConfig
SmartContractsStorageSimulate StorageConfig
Copy link
Contributor

Choose a reason for hiding this comment

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

SmartContractsStorageMock or SmartContractsStorageForTxSimulator instead SmartContractsStorageSimulate to each name here ?

@@ -33,7 +33,7 @@ func Test_newBlockProcessorCreatorForShard(t *testing.T) {
_, err := pcf.Create()
require.NoError(t, err)

bp, err := pcf.NewBlockProcessor(
bp, vmFactoryForSimulate, err := pcf.NewBlockProcessor(
Copy link
Contributor

Choose a reason for hiding this comment

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

vmFactoryMock or vmFactoryForTxSimulator ?

@@ -136,7 +137,7 @@ func Test_newBlockProcessorCreatorForMeta(t *testing.T) {
_, err = pcf.Create()
require.NoError(t, err)

bp, err := pcf.NewBlockProcessor(
bp, vmFactoryForSimulate, err := pcf.NewBlockProcessor(
Copy link
Contributor

Choose a reason for hiding this comment

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

vmFactoryMock or vmFactoryForTxSimulator ?

SmartContractsStorageSimulate: config.StorageConfig{
Cache: getLRUCacheConfig(),
DB: config.DBConfig{
FilePath: AddTimestampSuffix("SmartContractsStorageSimulate"),
Copy link
Contributor

Choose a reason for hiding this comment

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

SmartContractsStorageMock or SmartContractsStorageForTxSimulator instead SmartContractsStorageSimulate ?

}

smartContractStorageSimulate := pcf.config.SmartContractsStorageSimulate
Copy link
Contributor

Choose a reason for hiding this comment

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

SmartContractsStorageMock or SmartContractsStorageForTxSimulator instead SmartContractsStorageSimulate ?

@@ -189,6 +189,20 @@
MaxBatchSize = 100
MaxOpenFiles = 10

[SmartContractsStorageSimulate]
Copy link
Contributor

Choose a reason for hiding this comment

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

+1

adb.stackDebug = debug.Stack()
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Add comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

@@ -962,6 +965,17 @@ func (adb *AccountsDB) journalize(entry JournalEntry) {

adb.entries = append(adb.entries, entry)
log.Trace("accountsDB.Journalize", "new length", len(adb.entries))

if len(adb.entries) == 1 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why 1?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@iulianpascalau can you answer this ?

@LucianMincu
Copy link
Contributor

System test results

v1.2.22-dev-config to fix-boot-from-storage-after-import

  Object          Number       Verdict

Known ERRORS 77 Passed
New ERRORS 0 Passed
WARNs 325 Passed
PANICs 0 Passed

@LucianMincu LucianMincu merged commit 057598a into master Oct 22, 2021
@LucianMincu LucianMincu deleted the tx-similator-read-only-accounts-fixes branch October 22, 2021 16:17
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

4 participants