snapshots: add https #6354

cali-jumptrading · 2025-09-12T22:23:54Z

No description provided.

src/disco/bundle/fd_bundle_tile.c

ripatel-fd · 2025-11-21T17:11:07Z

src/discof/restore/utils/fd_sshttp.c

+http_connect_ssl( fd_sshttp_t * http,
+                  long          now ) {
+  if( FD_UNLIKELY( now>http->deadline ) ) {
+    FD_LOG_WARNING(("deadline exceeded during connect"));


Suggested change

FD_LOG_WARNING(("deadline exceeded during connect"));

FD_LOG_WARNING(( "deadline while connecting to HTTPS snapshot server" ));

Might also want to log the peer URL here

ripatel-fd · 2025-11-21T17:11:42Z

src/discof/restore/utils/fd_sshttp.c

+  if( FD_UNLIKELY( ssl_err!=1 ) ) {
+    int ssl_err_code = SSL_get_error( http->ssl, ssl_err );
+    if( FD_UNLIKELY( ssl_err_code!=SSL_ERROR_WANT_READ && ssl_err_code!=SSL_ERROR_WANT_WRITE ) ) {
+      FD_LOG_WARNING(( "SSL_connect failed (%d)", ssl_err ));


These WARNING logs should ideally be operator friendly.

e.g. error while downloading snapshot from https://: SSL_connect failed (%i-%s)

ripatel-fd · 2025-11-21T17:11:58Z

src/discof/restore/utils/fd_sshttp.c

+  if( FD_LIKELY( res<=0 ) ) {
+    int ssl_err_code = SSL_get_error( http->ssl, res );
+    if( FD_UNLIKELY( ssl_err_code!=SSL_ERROR_WANT_READ && ssl_err_code!=SSL_ERROR_WANT_WRITE && res!=0 ) ) {
+      FD_LOG_WARNING(( "SSL_shutdown failed (%d)", ssl_err_code ));


same here, warning log shoudl be operator friendly

ripatel-fd · 2025-11-21T17:12:07Z

src/discof/restore/utils/fd_sshttp.c

+    int ssl_err = SSL_get_error( http->ssl, read_res );
+
+    if( FD_UNLIKELY( ssl_err!=SSL_ERROR_WANT_READ && ssl_err!=SSL_ERROR_WANT_WRITE ) ) {
+      FD_LOG_WARNING(( "SSL_read failed (%d)", ssl_err ));


operator friendly warning log

ripatel-fd · 2025-11-21T17:12:13Z

src/discof/restore/utils/fd_sshttp.c

+    int ssl_err = SSL_get_error( http->ssl, write_res );
+
+    if( FD_UNLIKELY( ssl_err!=SSL_ERROR_WANT_READ && ssl_err!=SSL_ERROR_WANT_WRITE ) ) {
+      FD_LOG_WARNING(( "SSL_write failed (%d)", ssl_err ));


operator friendly warning log

ripatel-fd · 2025-11-21T17:12:35Z

src/discof/restore/utils/fd_sshttp.c

+  long sent = sendto( http->sockfd, buf, bufsz, 0, NULL, 0 );
+  if( FD_UNLIKELY( -1==sent && errno==EAGAIN ) ) return FD_SSHTTP_ADVANCE_AGAIN;
+  else if( FD_UNLIKELY( -1==sent ) ) {
+    FD_LOG_WARNING(( "sendto() failed (%d-%s)", errno, fd_io_strerror( errno ) ));


operator friendly warning log

ripatel-fd · 2025-11-21T17:12:49Z

src/discof/restore/utils/fd_sshttp.c

+        .fd = http->sockfd,
+        .events = POLLIN,
+      };
+      fd_syscall_poll( &pfd, 1 /*fds*/, 1 /*ms*/ );


check error code of syscall (FD_LOG_ERR if poll returns a weird errno)

ripatel-fd · 2025-11-21T17:13:35Z

src/discof/restore/utils/fd_ssresolve.c

+      int ssl_err = SSL_get_error( ssresolve->ssl, write_res );
+
+      if( FD_UNLIKELY( ssl_err!=SSL_ERROR_WANT_READ && ssl_err!=SSL_ERROR_WANT_WRITE ) ) {
+        FD_LOG_WARNING(( "SSL_write failed (%d)", ssl_err ));


operator friendly error log

ripatel-fd · 2025-11-21T17:14:07Z

src/discof/restore/utils/fd_ssresolve.c

+
+    read = (long)read_res;
+#else
+    FD_LOG_ERR(( "cannot use HTTPS without OpenSSL" ));


this build of Firedancer does not support OpenSSL, rebuild with deps.sh

or something along those lines, otherwise operator is going to be confused why OpenSSL was not fonud

src/discof/restore/utils/fd_ssresolve.c

ripatel-fd · 2025-11-21T17:14:20Z

src/discof/restore/utils/fd_ssresolve.c

+  if( FD_UNLIKELY( ssl_err!=1 ) ) {
+    int ssl_err_code = SSL_get_error( ssresolve->ssl, ssl_err );
+    if( FD_UNLIKELY( ssl_err_code!=SSL_ERROR_WANT_READ && ssl_err_code!=SSL_ERROR_WANT_WRITE ) ) {
+      FD_LOG_WARNING(( "SSL_connect failed (%d)", ssl_err_code ));


operator friendly error log

I'll address log-related comments in a follow up

src/discof/restore/fd_snapct_tile.c

src/discof/restore/fd_snapld_tile.c

src/waltz/openssl/fd_openssl_tile.c

changed my mind

github-actions · 2025-11-24T17:45:21Z

Performance Measurements ⏳

Suite	Baseline	New	Change
backtest `mainnet-368528500-perf` per slot	`0.05664 s`	`0.050926 s`	`-10.088%` ✅
backtest `mainnet-368528500-perf` snapshot load	`2.789 s`	`2.222 s`	`-20.330%` ✅
backtest `mainnet-368528500-perf` total elapsed	`56.639559 s`	`50.926228 s`	`-10.087%` ✅
`firedancer mem` usage with `mainnet.toml`	`1001.1 GiB`	`1001.23 GiB`	`0.013%` ✅

github-actions · 2025-11-24T17:55:04Z

Performance Measurements ⏳

Suite	Baseline	New	Change
backtest `mainnet-368528500-perf` per slot	`0.073687 s`	`0.07409 s`	`0.547%` ✅
backtest `mainnet-368528500-perf` snapshot load	`4.206 s`	`4 s`	`-4.898%` ✅
backtest `mainnet-368528500-perf` total elapsed	`73.686691 s`	`74.090247 s`	`0.548%` ✅
`firedancer mem` usage with `mainnet.toml`	`1001.1 GiB`	`1001.23 GiB`	`0.013%` ✅

Already addressed

Fixes incorrect usage of the OpenSSL 'static' feature which is intended for statically linking glibc. The static feature breaks multi-threaded use of OpenSSL, so it should be disabled. Note that libssl.a still gets built, so OpenSSL itself is still statically linked.

github-actions · 2025-11-24T21:56:53Z

Performance Measurements ⏳

Suite	Baseline	New	Change
backtest `mainnet-368528500-perf` per slot	`0.057834 s`	`0.050941 s`	`-11.919%` ✅
backtest `mainnet-368528500-perf` snapshot load	`2.739 s`	`1.668 s`	`-39.102%` ✅
backtest `mainnet-368528500-perf` total elapsed	`57.833887 s`	`50.940716 s`	`-11.919%` ✅
`firedancer mem` usage with `mainnet.toml`	`1001.1 GiB`	`1001.23 GiB`	`0.013%` ✅

ibhatt-jumptrading · 2025-11-25T20:34:59Z

src/discof/tower/fd_tower_tile.c

  FD_MCNT_SET( TOWER, HARD_FORKS_PRUNED,   ctx->metrics.hard_forks.pruned );

-  FD_MGAUGE_SET( TOWER, HARD_FORKS_ACTIVE,    ctx->metrics.hard_forks.active    );
-  FD_MGAUGE_SET( TOWER, HARD_FORKS_MAX_WIDTH, ctx->metrics.hard_forks.max_width );


was this intended?

yes, charlie forgot to remove this line after he removed the metric in metrics.xml

cali-jumptrading force-pushed the cali/add-https branch 4 times, most recently from af17034 to 29a54f9 Compare September 12, 2025 22:30

cali-jumptrading force-pushed the cali/add-https branch 24 times, most recently from a0bd347 to 3488f1f Compare September 23, 2025 16:31

cali-jumptrading marked this pull request as ready for review September 23, 2025 16:33

cali-jumptrading changed the title ~~snapshots: add https [WIP]~~ snapshots: add https Sep 23, 2025

ripatel-fd reviewed Nov 21, 2025

View reviewed changes

src/disco/bundle/fd_bundle_tile.c Outdated Show resolved Hide resolved

ripatel-fd reviewed Nov 21, 2025

View reviewed changes

ripatel-fd previously requested changes Nov 21, 2025

View reviewed changes

ripatel-fd self-requested a review November 21, 2025 17:20

ripatel-fd previously approved these changes Nov 21, 2025

View reviewed changes

cali-jumptrading dismissed stale reviews from ripatel-fd and amass-jump via 169f412 November 24, 2025 17:42

cali-jumptrading force-pushed the cali/add-https branch from 92a9bbd to 169f412 Compare November 24, 2025 17:42

snapshots: add https

d42f65d

cali-jumptrading force-pushed the cali/add-https branch from 169f412 to d42f65d Compare November 24, 2025 17:50

ripatel-fd previously approved these changes Nov 24, 2025

View reviewed changes

ripatel-fd dismissed their stale review via 984a284 November 24, 2025 21:40

mmcgee-jump approved these changes Nov 24, 2025

View reviewed changes

cali-jumptrading merged commit 4a5957a into main Nov 24, 2025
22 of 37 checks passed

cali-jumptrading deleted the cali/add-https branch November 24, 2025 22:39

ibhatt-jumptrading reviewed Nov 25, 2025

View reviewed changes

	FD_LOG_WARNING(("deadline exceeded during connect"));
	FD_LOG_WARNING(( "deadline while connecting to HTTPS snapshot server" ));

snapshots: add https #6354

snapshots: add https #6354

Uh oh!

Conversation

cali-jumptrading commented Sep 12, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Nov 24, 2025

Performance Measurements ⏳

Uh oh!

github-actions bot commented Nov 24, 2025

Performance Measurements ⏳

Uh oh!

github-actions bot commented Nov 24, 2025

Performance Measurements ⏳

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants