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

Do not set last retired fed p2sh script in new commitFederation impl #2842

Conversation

julia-zack
Copy link
Contributor

@julia-zack julia-zack commented Nov 7, 2024

Last-retired-federation script shouldn't be set in new commitFederation implementation, since we will set the to-be-active-federation after its validation.

Copy link

github-actions bot commented Nov 7, 2024

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

OpenSSF Scorecard

PackageVersionScoreDetails

Scanned Manifest Files

@julia-zack julia-zack force-pushed the update_svp_state_from_uc branch 2 times, most recently from efabbb1 to 99bdde2 Compare November 8, 2024 12:33
@julia-zack julia-zack force-pushed the remove_last_retired_p2sh_script_setting branch from 503aca7 to 0113e2a Compare November 11, 2024 16:22
@julia-zack julia-zack marked this pull request as ready for review November 11, 2024 16:25
@julia-zack julia-zack requested a review from a team as a code owner November 11, 2024 16:25
Copy link
Contributor

@jeremy-then jeremy-then left a comment

Choose a reason for hiding this comment

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

LGTM.

Base automatically changed from update_svp_state_from_uc to update_state_svp November 11, 2024 19:30
Copy link
Contributor

@apancorb apancorb left a comment

Choose a reason for hiding this comment

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

LGTM

@julia-zack julia-zack force-pushed the remove_last_retired_p2sh_script_setting branch from 0113e2a to 6983bb2 Compare November 13, 2024 18:36
Base automatically changed from update_state_svp to feature/powpeg_validation_protocol-phase4 November 13, 2024 20:32
@@ -733,12 +734,11 @@ private FederationChangeResponseCode commitPendingFederation(PendingFederation c
// set proposed federation
Federation proposedFederation = buildFederationFromPendingFederation(currentPendingFederation);
provider.setProposedFederation(proposedFederation);
setNewActiveFederationCreationBlockHeight();
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be done at this point? Shouldn't we set this value after the proposed federation is validated? If the validation fails we'd end up with an invalid value here, 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.

we discussed this when developing phase1, the idea was that the activation period remain the same. But yes it makes sense to do it after the validation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

@julia-zack julia-zack force-pushed the remove_last_retired_p2sh_script_setting branch from 6d61919 to 80b0d63 Compare November 19, 2024 17:47
@marcos-iov marcos-iov merged commit 3b02692 into feature/powpeg_validation_protocol-phase4 Nov 19, 2024
8 checks passed
@marcos-iov marcos-iov deleted the remove_last_retired_p2sh_script_setting branch November 19, 2024 18:03
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.

5 participants