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

Create new settleReleaseRequest method to perform common actions when creating pegouts #2692

Conversation

julia-zack
Copy link
Contributor

-Create a new settleReleaseRequest that perform all the needed actions whenever creating a pegout.
-Get rid of existing addToPegoutsWaitingForConfirmations method.

pegoutsWaitingForConfirmations.add(pegoutTransaction, rskExecutionBlockNumber, releaseCreationTxHash);
}

private void savePegoutTxSigHash(BtcTransaction pegoutTx) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this method was not modified, only moved to be closed where its being used.

@julia-zack julia-zack marked this pull request as ready for review August 19, 2024 19:18
@julia-zack julia-zack requested a review from a team as a code owner August 19, 2024 19:18
Copy link

Copy link
Contributor

@marcos-iov marcos-iov left a comment

Choose a reason for hiding this comment

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

Looking nice and clean. Well done!

}
} else {
pegoutWaitingForConfirmations.add(generatedTransaction, rskExecutionBlock.getNumber());
private void settleReleaseRequest(PegoutsWaitingForConfirmations pegoutsWaitingForConfirmations, BtcTransaction pegoutTransaction, Keccak256 releaseCreationTxHash, Coin requestedAmount) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we need to pass around the pegoutsWaitingForConfirmations, we can simply get it from the storageProvider right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, thats right, but the getPegoutsWaitingForConfirmations could throw an IOException that has to be handled all over the code. See for example in processPegoutsInBatch and processPegoutsIndividually, that the map is an input of the method already handled in processPegoutRequests method.

I thought it was better to do it this way to have less impact. But let me know what you think!

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, no big deal. Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks to you :)

@marcos-iov marcos-iov merged commit 2c6380e into feature/powpeg_validation_protocol Aug 20, 2024
6 checks passed
@marcos-iov marcos-iov deleted the add_settle_release_request_method branch August 20, 2024 12:46
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.

2 participants