Skip to content
This repository has been archived by the owner on Sep 26, 2019. It is now read-only.

[PAN-2949] Add rewind to block functionality #1814

Merged
merged 4 commits into from
Aug 2, 2019

Conversation

shemnon
Copy link
Contributor

@shemnon shemnon commented Aug 1, 2019

PR description

Retesteth will often request an in-process blockchain be rewound one block so
that it can attempt the same test with tweaked parameters. Add support for this.

shemnon added 2 commits August 1, 2019 10:51
Retesteth will often request an in-process blockchain be rewound one block so
that it can attempt the same test with tweaked parameters. Add support for this.
final BlockAddedEvent result = this.handleChainReorg(updater, block);
updater.commit();

if (result.isNewCanonicalHead()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is always true? Do you need the if / else?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

updater.commit();

if (result.isNewCanonicalHead()) {
chainHeader = block.getHeader();
Copy link
Contributor

Choose a reason for hiding this comment

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

(optional) You could dedupe this logic by adding a new method like updateCacheForNewCanonicalHead(Block newHead, Supplier<UInt256> td).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

final Block block = new Block(oldBlockHeader.get(), oldBlockBody.get());

final BlockchainStorage.Updater updater = blockchainStorage.updater();
final BlockAddedEvent result = this.handleChainReorg(updater, block);
Copy link
Contributor

Choose a reason for hiding this comment

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

This change ended up being surprisingly simple 🎉

}

final Optional<BlockHeader> oldBlockHeader = blockchainStorage.getBlockHeader(blockHash.get());
final Optional<BlockBody> oldBlockBody = blockchainStorage.getBlockBody(blockHash.get());
Copy link
Contributor

Choose a reason for hiding this comment

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

(optional) If we have the blockHash, should be safe to assume we have the header and body - we should probably throw an exception if they're missing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

// rewind it by 1 block
blockchain.rewindToBlock(targetHead.getHeader().getNumber());

// Check chain has not reorganized
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment looks wrong :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

reworded.

@shemnon shemnon merged commit 3115066 into PegaSysEng:master Aug 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants