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

Enable snapshots #1239

Merged
merged 3 commits into from
Jan 16, 2023
Merged

Enable snapshots #1239

merged 3 commits into from
Jan 16, 2023

Conversation

freshe4qa
Copy link
Contributor

Enable snapshots for testnet to enable validators to more quickly deploy nodes/run experiments

Overview

Checklist

  • New and updated code has appropriate documentation
  • New and updated code has new and/or updated testing
  • Required CI checks are passing
  • Visual proof for any user facing features like CLI or documentation updates
  • Linked issues closed with keywords

@freshe4qa
Enable snapshots
d56ad99 
Enable snapshots for testnet to enable validators to more quickly deploy nodes/run experiments
@MSevey MSevey requested review from a team and rootulp and removed request for a team January 15, 2023 07:50
@freshe4qa
Copy link
Contributor Author

Try using a newer version of "go".

evan-forbes
evan-forbes reviewed Jan 16, 2023
View reviewed changes
Copy link
Member

@evan-forbes evan-forbes left a comment

Choose a reason for hiding this comment

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

thanks for this!!

I believe all we have to do is to fix the linter before we merge, but I have yet to test this change.

@freshe4qa
Copy link
Contributor Author

Okey, cool

cmwaters
cmwaters requested changes Jan 16, 2023
View reviewed changes
Copy link
Contributor

@cmwaters cmwaters left a comment

Choose a reason for hiding this comment

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

Thanks @freshe4qa for the PR. Just needs a touch up and then we can merge it

cmd/celestia-appd/cmd/root.go Outdated Show resolved Hide resolved
@freshe4qa @cmwaters
Update cmd/celestia-appd/cmd/root.go
5bd3cd3 
baseapp.SetSnapshot(
			snapshotStore,
			snapshottypes.NewSnapshotOptions(
				cast.ToUint64(appOpts.Get(sdkserver.FlagStateSyncSnapshotInterval)),
				cast.ToUint32(appOpts.Get(sdkserver.FlagStateSyncSnapshotKeepRecent)),

Co-authored-by: Callum Waters <cmwaters19@gmail.com>
@MSevey MSevey requested a review from a team January 16, 2023 10:16
@freshe4qa
Copy link
Contributor Author

I corrected it, check.

@freshe4qa
Copy link
Contributor Author

I think we need to work on fixing the "lint".

@cmwaters
Copy link
Contributor

Yeah just did that now

@freshe4qa
Copy link
Contributor Author

Okey, cool.

cmwaters
cmwaters approved these changes Jan 16, 2023
View reviewed changes
Copy link
Contributor

@cmwaters cmwaters left a comment

Choose a reason for hiding this comment

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

Looks like everything is passing. Thanks for the PR

@cmwaters cmwaters merged commit fb92a69 into celestiaorg:main Jan 16, 2023
@freshe4qa
Copy link
Contributor Author

Always glad to help

@evan-forbes evan-forbes mentioned this pull request Jan 18, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@evan-forbes evan-forbes evan-forbes left review comments

@cmwaters cmwaters cmwaters approved these changes

@rootulp rootulp Awaiting requested review from rootulp rootulp is a code owner automatically assigned from celestiaorg/celestia-core

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@freshe4qa @cmwaters @evan-forbes