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

Pruning Tests and Bug Fixes #1132

Merged
merged 12 commits into from
Mar 19, 2020
Merged

Pruning Tests and Bug Fixes #1132

merged 12 commits into from
Mar 19, 2020

Conversation

AlexandraRoatis
Copy link
Contributor

Type of change

  • Bug fix.
  • New feature.
  • Enhancement.
  • Unit test.
  • Breaking change (a fix or feature that causes existing functionality to not work as expected).
  • Requires documentation update.

@AlexandraRoatis AlexandraRoatis added bug Something isn't working enhancement New feature or request unit tests labels Mar 17, 2020
@AlexandraRoatis AlexandraRoatis added this to the 1.5 milestone Mar 17, 2020
@AlexandraRoatis AlexandraRoatis self-assigned this Mar 17, 2020
@AlexandraRoatis AlexandraRoatis changed the title Aki 667 Pruning Tests and Bug Fixes Mar 17, 2020
@AlexandraRoatis AlexandraRoatis force-pushed the AKI-667 branch 2 times, most recently from 791d086 to 1ac3547 Compare March 17, 2020 21:37
@@ -512,10 +512,6 @@ public void flush() {
db.commit();
}
}
} else {
Copy link
Collaborator

Choose a reason for hiding this comment

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

What scenario we will see the databaseGroup equal to null ? Sometimes We can see this message during the seednode running, also the project test case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The databaseGroup is null when using a repository snapshot. The snapshot is always used when importing a side chain block. The message is really useless, cause it doesn't notify us of anything meaningful.

Copy link
Collaborator

@AionJayT AionJayT left a comment

Choose a reason for hiding this comment

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

LGTM, only one question for the databasegroup null case.

return null;
} else {
StakingBlock blockTemplate;
blockTemplateLock.lock();
try {
blockTemplate = blockchain.createStakingBlockTemplate(
mempool.getPendingTransactions(), signingPublicKey, newSeed, coinbase);
blockTemplate = blockchain.createStakingBlockTemplate(best, mempool.getPendingTransactions(), signingPublicKey, newSeed, coinbase);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we just use blockTemplateLock and the try...finally to include all of the method?
Same as the getMiningBlockTemplate

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added an extra commit for this change.

These are accounts for producing transactions that are not involved in staking.
JournalPruneDataSource already rolls back the side chain blocks.
The staking blocks were always created on top of the best block. This
is a valid setup for the staker tools but it prevents testing the
functionality with side chains.

Additionally, this is a bug fix for AionHub. The hub could cause incorrect
block creation due to the fact that it was calling the getBestBlock method
in the blockchain twice without synchronization.
This bug fix may relate to AKI-657.
The test is ignored for now pending the fix for AKI-677.
@AlexandraRoatis AlexandraRoatis merged commit ee8c73b into master Mar 19, 2020
@AlexandraRoatis AlexandraRoatis deleted the AKI-667 branch March 19, 2020 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request unit tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants