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.

Sign up for GitHub

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

Unit tests for the Blockstore #1129

Merged
merged 4 commits into from
Mar 13, 2020
Merged

Unit tests for the Blockstore #1129

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ed4d0db
Remove unused hasParentOnTheChain method
AlexandraRoatis Mar 11, 2020
bd38035
Add tests and bugfix for getBlockByHashWithInfo
AlexandraRoatis Mar 11, 2020
e11b29b
Add tests for getTwoGenerationBlocksByHashWithInfo
AlexandraRoatis Mar 10, 2020
5fb5e03
Add tests for getThreeGenerationBlocksByHashWithInfo
AlexandraRoatis Mar 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 4 additions & 9 deletions modAionImpl/src/org/aion/zero/impl/blockchain/AionBlockchainImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1289,6 +1289,7 @@ BlockContext createNewMiningBlockInternal(
return null;
} else {
Block[] blockFamily = repository.getBlockStore().getTwoGenerationBlocksByHashWithInfo(parentHdr.getParentHash());
Objects.requireNonNull(blockFamily[0]);
parentMiningBlock = blockFamily[0].getHeader();
parentMiningBlocksParent = blockFamily[1].getHeader();
diffCalculator = chainConfiguration.getUnityDifficultyCalculator();
Expand Down Expand Up @@ -1352,7 +1353,7 @@ private StakingBlock createNewStakingBlock(
forkUtility.setNonceForkResetDiff(newDiff);
} else {
Block[] blockFamily = repository.getBlockStore().getTwoGenerationBlocksByHashWithInfo(parentHdr.getParentHash());

Objects.requireNonNull(blockFamily[0]);
BlockHeader parentStakingBlock = blockFamily[0].getHeader();
BlockHeader parentStakingBlocksParent = blockFamily[1].getHeader();
parentSeed = ((StakingBlockHeader) parentStakingBlock).getSeed();
Expand Down Expand Up @@ -1666,12 +1667,10 @@ public boolean isValid(BlockHeader header) {
}

Block[] threeGenParents = repository.getBlockStore().getThreeGenerationBlocksByHashWithInfo(header.getParentHash());

if (threeGenParents == null) {
Block parentBlock = threeGenParents[0];
if (parentBlock == null) {
return false;
}

Block parentBlock = threeGenParents[0];
Block grandparentBlock = threeGenParents[1];
Block greatGrandparentBlock = threeGenParents[2];

Expand Down Expand Up @@ -2060,10 +2059,6 @@ public void dropImported(
}
}

public boolean hasParentOnTheChain(Block block) {
return getParent(block.getHeader()) != null;
}

public TransactionStore getTransactionStore() {
return transactionStore;
}
Expand Down
39 changes: 22 additions & 17 deletions modAionImpl/src/org/aion/zero/impl/db/AionBlockStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -558,9 +558,10 @@ boolean isBlockStored(byte[] hash, long number) {
}

/**
* Retrieve block with unity protocol info
* @param hash given hash of the block
* @return the block data has matched hash with unity protocol info
* Retrieve a block with unity protocol info.
*
* @param hash the hash of the requested block
* @return the block data for the given hash with unity protocol info
*/
public Block getBlockByHashWithInfo(byte[] hash) {
if (hash == null) {
Expand Down Expand Up @@ -1391,18 +1392,21 @@ void redoIndexWithoutSideChains(Block block) {
}

/**
* Retrieve three generation blocks with unity protocol info with one lock.
* @param hash given hash of the block
* @return the 3 generation block data have matched hash with unity protocol info. Block[0] is the parent block,
* Block[1] is the grandParent block, and Block[2] is the greatParentBlock. The return might only contain the parent
* block and still return the 3-elements array.
* Retrieves three generation blocks with unity protocol info.
* <p>
* Always returns a 3-element array. If the blocks cannot be retrieved the array will contain null values.
* Block[0] is the parent block and has the given hash. Block[1] is the grandparent block.
* Block[2] is the great grandparent block.
*
* @param hash the hash of the parent block
* @return the retrieved three generation blocks with unity protocol info
*/
public final Block[] getThreeGenerationBlocksByHashWithInfo(byte[] hash) {
Block[] blockFamily = new Block[] { null, null, null};
if (hash == null) {
return null;
return blockFamily;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this change will let AionBlockChainImpl.isValid() throw null exception. Please have a check.

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 updated the null check in AionBlockchainImpl.isValid to exit if the parent block is null.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Cool.

}

Block[] blockFamily = new Block[] { null, null, null};
lock.lock();

try {
Expand All @@ -1428,18 +1432,19 @@ public final Block[] getThreeGenerationBlocksByHashWithInfo(byte[] hash) {
}

/**
* Retrieve two generation blocks with unity protocol info with one lock.
* Retrieves two generation blocks with unity protocol info.
* <p>
* Always returns a 2-element array. If the blocks cannot be retrieved the array will contain null values.
* Block[0] is the parent block and has the given hash. Block[1] is the grandparent block.
*
* @param hash given hash of the block
* @return the 2 generation block data have matched hash with unity protocol info. Block[0] is
* the parent block, Block[1] is the grandParent block. The return might only contain the
* parent block and still return the 2-elements array.
* @param hash the hash of the parent block
* @return the retrieved two generation blocks with unity protocol info
*/
public final Block[] getTwoGenerationBlocksByHashWithInfo(byte[] hash) {
Block[] blockFamily = new Block[] { null, null};
if (hash == null) {
return null;
return blockFamily;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Will let AionBlockChainImpl.createNewMiningBlockInternal() throw NPE

Copy link
Contributor Author

@AlexandraRoatis AlexandraRoatis Mar 12, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

It would already throw an NPE because the blockFamily is accessed without a null check. The reason for the NPE differs but it will be thown in either case. If that happens I think we should allows the failure and fix the cause when we find one.

Copy link
Collaborator

Choose a reason for hiding this comment

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

So We should just throw NPE if the input hash equal to null. instead of return values (or return null)

Same as the other method.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok. I made the update.

}
Block[] blockFamily = new Block[] { null, null};

lock.lock();

Expand Down
4 changes: 2 additions & 2 deletions modAionImpl/src/org/aion/zero/impl/types/AbstractBlock.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
public abstract class AbstractBlock implements Block {

/** use for cli tooling */
Boolean mainChain;
boolean mainChain = false;

// set from BlockInfos in index database
protected BigInteger totalDifficulty;
Expand Down Expand Up @@ -93,7 +93,7 @@ public void setTotalDifficulty(BigInteger totalDifficulty) {

@Override
public boolean isMainChain() {
return mainChain == null ? true : mainChain;
return mainChain;
}

@Override
Expand Down
4 changes: 1 addition & 3 deletions modAionImpl/src/org/aion/zero/impl/types/AionBlock.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,9 +338,7 @@ public String toString() {
toStringBuff.append(" total difficulty=").append(totalDifficulty).append("\n");
}

if (mainChain != null) {
toStringBuff.append(" mainChain=").append(mainChain ? "yes" : "no").append("\n");
}
toStringBuff.append(" mainChain=").append(mainChain ? "yes" : "no").append("\n");

if (!getTransactionsList().isEmpty()) {
toStringBuff
Expand Down
4 changes: 1 addition & 3 deletions modAionImpl/src/org/aion/zero/impl/types/StakingBlock.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,9 +325,7 @@ public String toString() {
toStringBuff.append(" total difficulty=").append(totalDifficulty).append("\n");
}

if (mainChain != null) {
toStringBuff.append(" mainChain=").append(mainChain ? "yes" : "no").append("\n");
}
toStringBuff.append(" mainChain=").append(mainChain ? "yes" : "no").append("\n");

if (!getTransactionsList().isEmpty()) {
toStringBuff.append("Txs [\n");
Expand Down
Loading