-
Notifications
You must be signed in to change notification settings - Fork 130
[PAN-2949] Add rewind to block functionality #1814
Conversation
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()) { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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(); |
There was a problem hiding this comment.
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)
.
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
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()); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment looks wrong :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reworded.
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.