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

Jump to bottom

[NIT-2628] Add option to enable fast confirmation #2434

Merged
merged 32 commits into from
Jul 23, 2024
Merged

[NIT-2628] Add option to enable fast confirmation #2434

merged 32 commits into from
Jul 23, 2024

Conversation

amsanghi
Copy link
Contributor

@amsanghi amsanghi commented Jun 26, 2024 •
edited
Loading

Backport of bold OffchainLabs/bold#673

@cla-bot cla-bot bot added the s Automatically added by the CLA bot if the creator of a PR is registered as having signed the CLA. label Jun 26, 2024
@amsanghi amsanghi changed the title [NIT-2628] Add option to enable fast confirmation [NIT-2628] Add option to enable fast confirmation (Safe integration pending) Jun 26, 2024
@amsanghi amsanghi changed the title [NIT-2628] Add option to enable fast confirmation (Safe integration pending) [NIT-2628] Add option to enable fast confirmation Jun 27, 2024
@amsanghi amsanghi marked this pull request as ready for review June 27, 2024 14:48
@PlasmaPower
Copy link
Collaborator

From the safe-smart-account README https://github.com/safe-global/safe-smart-account

⚠️ This branch contains changes that are under development To use the latest audited version make sure to use the correct commit. The tagged versions that are used by the Safe team can be found in the releases.

I think we should pin it to the latest release, 192c7dc and update the pin for bold to that as well

@amsanghi
Copy link
Contributor Author

amsanghi commented Jul 9, 2024

From the safe-smart-account README https://github.com/safe-global/safe-smart-account

⚠️ This branch contains changes that are under development To use the latest audited version make sure to use the correct commit. The tagged versions that are used by the Safe team can be found in the releases.

I think we should pin it to the latest release, 192c7dc and update the pin for bold to that as well

Thanks for catching this, updated! (Will update in bold as well)

@amsanghi amsanghi mentioned this pull request Jul 9, 2024
Merged
PlasmaPower
PlasmaPower reviewed Jul 18, 2024
View reviewed changes
Copy link
Collaborator

@PlasmaPower PlasmaPower left a comment

Choose a reason for hiding this comment

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

None of these comments are critical, but I think it'd be good to fix the approved hashes query before merging at least

staker/staker.go Outdated Show resolved Hide resolved
staker/staker.go Outdated Show resolved Hide resolved
staker/staker.go Outdated Show resolved Hide resolved
staker/staker.go Outdated Show resolved Hide resolved
system_tests/staker_test.go Outdated Show resolved Hide resolved
PlasmaPower
PlasmaPower requested changes Jul 19, 2024
View reviewed changes
staker/fast_confirm.go Outdated Show resolved Hide resolved
staker/fast_confirm.go Outdated Show resolved Hide resolved
@tsahee tsahee removed their request for review July 23, 2024 00:04
PlasmaPower
PlasmaPower approved these changes Jul 23, 2024
View reviewed changes
Copy link
Collaborator

@PlasmaPower PlasmaPower left a comment

Choose a reason for hiding this comment

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

LGTM

@PlasmaPower PlasmaPower merged commit 9197ebe into master Jul 23, 2024
15 checks passed
@PlasmaPower PlasmaPower deleted the fast_confirmation branch July 23, 2024 04:46
@PlasmaPower PlasmaPower mentioned this pull request Jul 24, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PlasmaPower PlasmaPower PlasmaPower approved these changes

@rauljordan rauljordan Awaiting requested review from rauljordan

Assignees
No one assigned
Labels
design-approved s Automatically added by the CLA bot if the creator of a PR is registered as having signed the CLA.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@amsanghi @PlasmaPower