feat: hash checksums + download urls for snapshot restore and create #201

BrendanCoughlan5 · 2025-01-27T17:38:51Z

hash checksums + download urls for snapshot restore and create, also cleaned up/ logs/ readme/ docs

README.md

docs/snapshots_docs.md

pkg/snapshot/snapshot.go

pkg/snapshot/snapshot_test.go

…nd create

docs/src/content/docs/running/snapshots.md

internal/config/config.go

pkg/snapshot/snapshot.go

seanmcgary · 2025-01-30T20:30:05Z

pkg/snapshot/snapshot.go

+			hashFileName := inputFileName + ".sha256sum"
+			hashFile, err := downloadFile(inputUrl+".sha256sum", hashFileName)
+			if err != nil {
+				return fmt.Errorf("failed to download snapshot hash: %w", err)


Do we want to differentiate between an problem downloading the file and an error like a 404 where the file simply doesnt exist? If it doesnt exist, what do you think is the best thing to do?

When using the repo_metadata/ manifest, if there's a 404 and you can't find the file, then you should go through older versions, i.e. curl -I through newest -> oldest, then download it, i think this would be handled no in the downloadFile but in a function interpreting the manifest to find the file.

also did the errors.Wrap and the downloadFile gives a 404 to be more explicit

potentially you could give an error that the hash file isn't there, no verification happened and then proceed with restoring the snapshot, but i think that might be dangerous/ security wouldn't like that? Maybe for testnet that's fine to allow, i'm a bit hesitant to allow restore snapshot without

pkg/snapshot/snapshot.go

pkg/snapshot/snapshot_test.go

docs/src/content/docs/running/snapshots.md

pkg/snapshot/snapshot.go

seanmcgary

Selected the wrong option on accident...see above feedback for changes needed.

internal/config/config.go

seanmcgary · 2025-02-03T15:52:04Z

Ran the restore-snapshot function and encountered a strange error. It seems to have downloaded the file completely, but then say's it received a 404 after the fact (which isnt possible):

downloading tmp/sidecar-testnet-holesky_v3.0.0-rc.1_public_20250122.dump 100% [========================================] Error: failed to restore snapshot: Error downloading snapshot from 'https://eigenlayer-sidecar.s3.us-east-1.amazonaws.com/snapshots/testnet-holesky/sidecar-testnet-holesky_v3.0.0-rc.1_public_20250122.dump': downloading error, received status code 404

It also didnt clean up the file that was created, which it probably should.

arguments:

go run main.go restore-snapshot
    --ethereum.rpc-url="https://winter-white-crater.ethereum-holesky.quiknode.pro/1b1d75c4ada73b7ad98e1488880649d4ea637733/" \
    --chain="preprod" \
    --database.host="localhost" \
    --database.port="5432" \
    --database.user="seanmcgary" \
    --database.password="" \
    --database.db_name="sidecar_test_restore" \
        --ethereum.use_native_batch_call=false \
        --snapshot.input=https://eigenlayer-sidecar.s3.us-east-1.amazonaws.com/snapshots/testnet-holesky/sidecar-testnet-holesky_v3.0.0-rc.1_public_20250122.dump \
        --database.schema_name=public

As a sidenote, when we encounter an error during execution, can we change it so that the usage message isnt printed? It adds a lot of noise and should really only be printed if the inputs are incorrect.

BrendanCoughlan5 · 2025-02-03T21:01:52Z

Ran the restore-snapshot function and encountered a strange error. It seems to have downloaded the file completely, but then say's it received a 404 after the fact (which isnt possible):
downloading tmp/sidecar-testnet-holesky_v3.0.0-rc.1_public_20250122.dump 100% [========================================] Error: failed to restore snapshot: Error downloading snapshot from 'https://eigenlayer-sidecar.s3.us-east-1.amazonaws.com/snapshots/testnet-holesky/sidecar-testnet-holesky_v3.0.0-rc.1_public_20250122.dump': downloading error, received status code 404
It also didnt clean up the file that was created, which it probably should.

arguments:
go run main.go restore-snapshot
    --ethereum.rpc-url="https://winter-white-crater.ethereum-holesky.quiknode.pro/1b1d75c4ada73b7ad98e1488880649d4ea637733/" \
    --chain="preprod" \
    --database.host="localhost" \
    --database.port="5432" \
    --database.user="seanmcgary" \
    --database.password="" \
    --database.db_name="sidecar_test_restore" \
        --ethereum.use_native_batch_call=false \
        --snapshot.input=https://eigenlayer-sidecar.s3.us-east-1.amazonaws.com/snapshots/testnet-holesky/sidecar-testnet-holesky_v3.0.0-rc.1_public_20250122.dump \
        --database.schema_name=public
As a sidenote, when we encounter an error during execution, can we change it so that the usage message isnt printed? It adds a lot of noise and should really only be printed if the inputs are incorrect.

Fixed to be clear it's the filesum the hash that doesn't exist.

Try with the --snapshot.verify-input=false flag

It should now give something like : {"level":"fatal","ts":"2025-02-03T12:53:34.148-0800","caller":"cmd/restoreSnapshot.go:44","msg":"failed to restore snapshot","error":"snapshot hash file not found at 'https://eigenlayer-sidecar.s3.us-east-1.amazonaws.com/snapshots/testnet-holesky/sidecar-testnet-holesky_v3.0.0-rc.1_public_20250122.dump.sha256sum'. Ensure the file exists or set --verify-input=false to skip verification","stacktrace":"github.com/Layr-Labs/sidecar/cmd.init.func4\n\t/Users/brendan/Documents/GitHub/sidecar/cmd/restoreSnapshot.go:44\ngithub.com/spf13/cobra.(*Command).execute\n\t/Users/brendan/go/pkg/mod/github.com/spf13/[email protected]/command.go:985\ngithub.com/spf13/cobra.(*Command).ExecuteC\n\t/Users/brendan/go/pkg/mod/github.com/spf13/[email protected]/command.go:1117\ngithub.com/spf13/cobra.(*Command).Execute\n\t/Users/brendan/go/pkg/mod/github.com/spf13/[email protected]/command.go:1041\ngithub.com/Layr-Labs/sidecar/cmd.Execute\n\t/Users/brendan/Documents/GitHub/sidecar/cmd/root.go:19\nmain.main\n\t/Users/brendan/Documents/GitHub/sidecar/main.go:8\nruntime.main\n\t/opt/homebrew/Cellar/go/1.23.4/libexec/src/runtime/proc.go:272"}

cmd/createSnapshot.go

cmd/restoreSnapshot.go

pkg/snapshot/snapshot.go

seanmcgary · 2025-02-05T15:24:32Z

pkg/snapshot/snapshot.go

+			}
+		}
+
+		// Download the snapshot file and assign to s.cfg.Input


except we're using inputUrl not s.cfg.Input. Comments should explain why, not what and/or clarify potentially complex code/edgecases

seanmcgary · 2025-02-05T15:24:56Z

pkg/snapshot/snapshot.go

+		}
+
+		// Download the snapshot file and assign to s.cfg.Input
+		fileName, err := getFileNameFromURL(inputUrl)


Comment says we're downloading the file but this function is not downloading the file. Recommend removing the comment

seanmcgary · 2025-02-05T15:26:51Z

pkg/snapshot/snapshot.go

+	if err != nil {
+		return errors.Wrap(err, fmt.Sprintf("failed to resolve input file path '%s'", s.cfg.Input))
+	}
+	s.cfg.Input = resolvedFilePath


Same as above, we should not be overwriting config values

seanmcgary · 2025-02-05T15:28:22Z

pkg/snapshot/snapshot.go

 	}

-	info, err := os.Stat(s.cfg.InputFile)
+	// s.cfg.input is resolved from a url in resolveAndDownloadRestoreInput()


if we're having to leave comments because the meaning of our config values changes during runtime, we need to modify the behavior to prevent that. config values should be immutable in nature

pkg/snapshot/snapshot_test.go

BrendanCoughlan5 · 2025-02-06T09:16:30Z

#201 (comment)

downloadFile is intended to be used to download an individual file, like you downloadFile(snapshot), then downloadFile(snapshotHash)

BrendanCoughlan5 requested a review from a team as a code owner January 27, 2025 17:38

BrendanCoughlan5 requested review from seanmcgary and 0xrajath January 27, 2025 17:38

BrendanCoughlan5 changed the title ~~feat: introduce hash checksums + download urls for snapshot restore a…~~ feat: hash checksums + download urls for snapshot restore and create Jan 27, 2025

BrendanCoughlan5 force-pushed the hashes_and_snapshot_download branch from 1bac8ff to 43ca345 Compare January 27, 2025 17:41

seanmcgary requested changes Jan 27, 2025

View reviewed changes

BrendanCoughlan5 force-pushed the hashes_and_snapshot_download branch from 17209f2 to 443cb30 Compare January 29, 2025 10:37

BrendanCoughlan5 self-assigned this Jan 29, 2025

BrendanCoughlan5 force-pushed the hashes_and_snapshot_download branch from acb5f62 to d215762 Compare January 30, 2025 19:59

BrendanCoughlan5 added 9 commits January 30, 2025 12:01

feat: introduce hash checksums + download urls for snapshot restore a…

40e086a

…nd create

removed output file as a specific param

d0ff279

updated for input to have the file and the url as well in one

4af5c7f

clean up NewSnapshotService, put into functions

3bef68c

logs use zap

0e9ad7e

clean logs up

f129199

update docs :)

521e931

CLI UX restore-snapshot downloading bar

ab02b6a

readme update to download from url

28d5544

BrendanCoughlan5 force-pushed the hashes_and_snapshot_download branch from d215762 to 28d5544 Compare January 30, 2025 20:02

seanmcgary requested changes Jan 30, 2025

View reviewed changes

adjustments

f6f529e

BrendanCoughlan5 mentioned this pull request Jan 31, 2025

[merge pending CNAME] docs: sidecar docs a bit clearer, add snapshot website #192

Closed

better errors

51c8b82

seanmcgary approved these changes Feb 3, 2025

View reviewed changes

seanmcgary self-requested a review February 3, 2025 14:49

seanmcgary requested changes Feb 3, 2025

View reviewed changes

seanmcgary reviewed Feb 3, 2025

View reviewed changes

internal/config/config.go Show resolved Hide resolved

BrendanCoughlan5 added 2 commits February 3, 2025 08:07

remove comment

43fbdea

cleanups

d05999b

BrendanCoughlan5 added 2 commits February 3, 2025 09:31

update documentation/ remove schema transformation script

c1c6617

better handling of 404 error

5a456a8

seanmcgary requested changes Feb 5, 2025

View reviewed changes

fix createSnapshot/ restoreSnapshot logs

3929046

BrendanCoughlan5 force-pushed the hashes_and_snapshot_download branch from f149b27 to 3929046 Compare February 6, 2025 07:07

BrendanCoughlan5 added 2 commits February 5, 2025 23:10

removed ftp references in comments

670dce4

update to use isHttpURL

4bd2504

shell out for the sha256sum cli as well

18827a7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: hash checksums + download urls for snapshot restore and create #201

feat: hash checksums + download urls for snapshot restore and create #201

BrendanCoughlan5 commented Jan 27, 2025 •

edited

Loading

seanmcgary Jan 30, 2025

BrendanCoughlan5 Jan 30, 2025 •

edited

Loading

BrendanCoughlan5 Jan 30, 2025

BrendanCoughlan5 Feb 1, 2025

seanmcgary left a comment

seanmcgary commented Feb 3, 2025 •

edited

Loading

BrendanCoughlan5 commented Feb 3, 2025 •

edited

Loading

seanmcgary Feb 5, 2025

seanmcgary Feb 5, 2025

seanmcgary Feb 5, 2025

seanmcgary Feb 5, 2025

BrendanCoughlan5 commented Feb 6, 2025

feat: hash checksums + download urls for snapshot restore and create #201

Are you sure you want to change the base?

feat: hash checksums + download urls for snapshot restore and create #201

Conversation

BrendanCoughlan5 commented Jan 27, 2025 • edited Loading

seanmcgary Jan 30, 2025

Choose a reason for hiding this comment

BrendanCoughlan5 Jan 30, 2025 • edited Loading

Choose a reason for hiding this comment

BrendanCoughlan5 Jan 30, 2025

Choose a reason for hiding this comment

BrendanCoughlan5 Feb 1, 2025

Choose a reason for hiding this comment

seanmcgary left a comment

Choose a reason for hiding this comment

seanmcgary commented Feb 3, 2025 • edited Loading

BrendanCoughlan5 commented Feb 3, 2025 • edited Loading

seanmcgary Feb 5, 2025

Choose a reason for hiding this comment

seanmcgary Feb 5, 2025

Choose a reason for hiding this comment

seanmcgary Feb 5, 2025

Choose a reason for hiding this comment

seanmcgary Feb 5, 2025

Choose a reason for hiding this comment

BrendanCoughlan5 commented Feb 6, 2025

BrendanCoughlan5 commented Jan 27, 2025 •

edited

Loading

BrendanCoughlan5 Jan 30, 2025 •

edited

Loading

seanmcgary commented Feb 3, 2025 •

edited

Loading

BrendanCoughlan5 commented Feb 3, 2025 •

edited

Loading