WIP: changes to merge gossip2 branch #5154

ravyu-jump · 2025-05-16T21:50:03Z

setup type defs and fn defintions for parsing
parsers for ping and pong
progress
rx ping/pong, tx ping, ping/pong serializers, helper functions, coalesce ping and pong into one type

…sce ping and pong into one type

mmcgee-jump · 2025-05-18T19:11:59Z

src/discof/gossip/fd_gossip_tile.c

@@ -169,6 +171,7 @@ out1( fd_topo_t const *      topo,
  }

  if( FD_UNLIKELY( idx==ULONG_MAX ) ) FD_LOG_ERR(( "tile %s:%lu had no output link named %s", tile->name, tile->kind_id, name ));
+  if( opt_tile_out_idx ) *opt_tile_out_idx = idx;


This is already returned as result->idx

mmcgee-jump · 2025-05-18T19:12:55Z

src/discof/gossip/fd_gossip_tile.c

+  *ctx->sign_out   = out1( topo, tile, "gossip_sign", &sign_out_tile_idx );
+
+  fd_topo_link_t * sign_in  = &topo->links[ tile->in_link_id [ sign_in_tile_idx  ] ];
+  fd_topo_link_t * sign_out = &topo->links[ tile->out_link_id[ sign_out_tile_idx ] ];


Suggested change

fd_topo_link_t * sign_out = &topo->links[ tile->out_link_id[ sign_out_tile_idx ] ];

fd_topo_link_t * sign_out = &topo->links[ tile->out_link_id[ ctx->sign_out->idx ] ];

mmcgee-jump · 2025-05-18T19:13:14Z

src/discof/gossip/fd_gossip_tile.c

+  fd_topo_link_t * sign_in  = &topo->links[ tile->in_link_id [ sign_in_tile_idx  ] ];
+  fd_topo_link_t * sign_out = &topo->links[ tile->out_link_id[ sign_out_tile_idx ] ];
+  if( fd_keyguard_client_join( fd_keyguard_client_new( ctx->keyguard_client,
+                                                                 sign_out->mcache, 


nit: alignment

mmcgee-jump · 2025-05-18T19:13:39Z

src/flamenco/gossip/fd_crds.h

@@ -2,6 +2,7 @@
 #define HEADER_fd_src_flamenco_gossip_fd_crds_h

 #include "../../util/fd_util.h"
+#include "../../util/net/fd_net_headers.h"


Not needed by the header, only add include to the .h file if the header uses it, this should be in the .c

mmcgee-jump · 2025-05-18T19:16:27Z

src/flamenco/gossip/fd_gossip.c

-    }
+verify_signatures( fd_gossip_message_t const *  message,
+                   uchar const *                payload ) {
+  if( FD_UNLIKELY( !( !!message->has_signable_data || !!message->crds_cnt ) ) )


Don't need this line, it doesn't do anything.

mmcgee-jump · 2025-05-18T19:16:44Z

src/flamenco/gossip/fd_gossip.c

+  if( FD_UNLIKELY( !( !!message->has_signable_data || !!message->crds_cnt ) ) )
+    return FD_GOSSIP_RX_VERIFY_NO_SIGNABLE_DATA;
+
+  fd_sha512_t sha[1];


As an argument, from the fd_gossip_t

mmcgee-jump · 2025-05-18T19:17:23Z

src/flamenco/gossip/fd_gossip.c

+     data */
+  if( FD_UNLIKELY( message->has_signable_data ) ) {
+    /* TODO: Special case for prune */
+    err = fd_ed25519_verify( payload + message->signable_data_offset,


style nit

Suggested change

err = fd_ed25519_verify( payload + message->signable_data_offset,

err = fd_ed25519_verify( payload+message->signable_data_offset,

mmcgee-jump · 2025-05-18T19:18:33Z

src/flamenco/gossip/fd_gossip.c

-    } else {
-      failed_inserts_append( gossip, pull_response->values[ i ] );
-    }
+  if( FD_UNLIKELY( err != FD_ED25519_SUCCESS ) ) return err;


kbowers style nit, :) please apply everywhere

Suggested change

if( FD_UNLIKELY( err != FD_ED25519_SUCCESS ) ) return err;

if( FD_UNLIKELY( err!=FD_ED25519_SUCCESS ) ) return err;

mmcgee-jump · 2025-05-18T19:20:04Z

src/flamenco/gossip/fd_gossip.c


 static int
 rx_prune( fd_gossip_t *             gossip,
          fd_gossip_prune_t const * prune,
          long                      now ) {
-  if( FD_UNLIKELY( now-500L*1000L*1000L>prune->data->wallclock ) ) return FD_GOSSIP_RX_ERR_PRUNE_TIMEOUT;
-  else if( FD_UNLIKELY( !memcmp( gossip->identity_pubkey, prune->data->destination, 32UL ) ) ) return FD_GOSSIP_RX_ERR_PRUNE_DESTINATION;
+  if( FD_UNLIKELY( now-500L*1000L*1000L>(long)prune->wallclock ) ) return FD_GOSSIP_RX_PRUNE_ERR_TIMEOUT;


prune wallclock should already be a long, and unitize to nanos, I'd rename wallclock_nanos

mmcgee-jump · 2025-05-18T19:20:41Z

src/flamenco/gossip/fd_gossip.c


 static int
 rx_prune( fd_gossip_t *             gossip,
          fd_gossip_prune_t const * prune,
          long                      now ) {
-  if( FD_UNLIKELY( now-500L*1000L*1000L>prune->data->wallclock ) ) return FD_GOSSIP_RX_ERR_PRUNE_TIMEOUT;
-  else if( FD_UNLIKELY( !memcmp( gossip->identity_pubkey, prune->data->destination, 32UL ) ) ) return FD_GOSSIP_RX_ERR_PRUNE_DESTINATION;
+  if( FD_UNLIKELY( now-500L*1000L*1000L>(long)prune->wallclock ) ) return FD_GOSSIP_RX_PRUNE_ERR_TIMEOUT;


Suggested change

if( FD_UNLIKELY( now-500L*1000L*1000L>(long)prune->wallclock ) ) return FD_GOSSIP_RX_PRUNE_ERR_TIMEOUT;

FD_GOSSIP_RX_PRUNE_ERR_STALE;

Is probably a little more clear than TIMEOUT

mmcgee-jump · 2025-05-18T19:21:29Z

src/flamenco/gossip/fd_ping_tracker.c

@@ -201,6 +201,16 @@ fd_ping_tracker_join( void * shpt ) {
  return ping_tracker;
 }

+static void


Suggested change

static void

static inline void

mmcgee-jump · 2025-05-18T19:22:05Z

src/flamenco/gossip/fd_ping_tracker.c

@@ -201,6 +201,16 @@ fd_ping_tracker_join( void * shpt ) {
  return ping_tracker;
 }

+static void
+hash_ping_token( uchar const *         ping_token,
+                 uchar *               expected_pong_token,


Suggested change

uchar * expected_pong_token,

uchar expected_pong_token[ static 32UL ],

mmcgee-jump · 2025-05-18T19:24:11Z

src/flamenco/gossip/fd_gossip_types.h

  uchar from[ 32UL ];
-  uchar hash[ 32UL ];
+  ulong prunes_len;
+  uchar prunes[ 33UL ][ 32UL ]; /* 33 pubkeys fit in MTU (rounded down) */


Can you add detail on the calculation to get 33 here?

mmcgee-jump · 2025-05-18T19:26:06Z

src/flamenco/gossip/fd_gossip_msg.h

+typedef struct fd_gossip_message fd_gossip_message_t;
+
+void
+fd_gossip_msg_init( fd_gossip_message_t * msg );


Not needed, zero initializing these is a performance bug

mmcgee-jump · 2025-05-18T19:29:18Z

src/flamenco/gossip/fd_gossip.c

-  if( FD_UNLIKELY( error ) ) return error;
+  uchar * gossip_payload;
+  ulong   gossip_payload_sz;
+  fd_ip4_port_t peer_address;


Suggested change

fd_ip4_port_t peer_address;

fd_ip4_port_t peer_address[ 1 ];

mmcgee-jump · 2025-05-18T19:43:03Z

src/flamenco/gossip/fd_gossip.c

+                                      &peer_pubkey,
+                                      &peer_address,
+                                      &ping_token ) ) {
+    fd_gossip_message_t * message = new_outgoing( gossip );


No need to construct this message, this is just additional copies, you can serialize directly from the components.

mmcgee-jump · 2025-05-18T19:43:59Z

src/flamenco/gossip/fd_gossip.c

+  fd_ip4_hdr_t const * ip4 = (fd_ip4_hdr_t const *)( (ulong)eth + sizeof(fd_eth_hdr_t) );
+  fd_udp_hdr_t const * udp = (fd_udp_hdr_t const *)( (ulong)ip4 + FD_IP4_GET_LEN( *ip4 ) );
+
+  if( FD_UNLIKELY( (ulong)udp+sizeof(fd_udp_hdr_t) > (ulong)eth+data_sz ) ) return FD_GOSSIP_RX_ERR_NETWORK_HDRS;


I'd FD_LOG_ERR on these, gossip expects well formed network packets from the net tile.

mmcgee-jump · 2025-05-18T19:44:20Z

src/flamenco/gossip/fd_gossip.h

+#define FD_GOSSIP_RX_OK                         (0)
+
+#define FD_GOSSIP_RX_PARSE_ERR                  (1)
+#define FD_GOSSIP_RX_ERR_NETWORK_HDRS           (2)


Not needed, see other comment

mmcgee-jump · 2025-05-18T19:46:32Z

src/flamenco/gossip/fd_gossip_msg_parse.c

+  /* Extract enum discriminant/tag (4b encoded) */
+  uint tag = 0;
+  CHECK_LEFT( 4UL                            );   tag = payload[ i ];     i+=4;
+  CHECK(      tag<FD_GOSSIP_MESSAGE_END      );


Suggested change

CHECK( tag<FD_GOSSIP_MESSAGE_END );

CHECK( tag<=FD_GOSSIP_MESSAGE_LAST );

mmcgee-jump · 2025-05-18T19:48:40Z

src/flamenco/gossip/fd_crds_value.h

+#define FD_GOSSIP_CLIENT_FD     (2)
+#define FD_GOSSIP_CLIENT_AGAVE  (3)
+
+struct fd_gossip_contact_info {


I would namespace these,

Suggested change

struct fd_gossip_contact_info {

struct fd_gossip_message_contact_info {

mmcgee-jump · 2025-05-18T19:51:05Z

src/flamenco/gossip/fd_ping_tracker.h

+   Assumes both token and hash are the starting address of a 32byte region of
+   memory */
+void
+fd_ping_tracker_hash_ping_token( uchar const * token,


Suggested change

fd_ping_tracker_hash_ping_token( uchar const * token,

fd_ping_tracker_respond( uchar const * token,

mmcgee-jump · 2025-05-18T19:51:30Z

src/flamenco/gossip/fd_gossip.c

+  fd_ping_tracker_hash_ping_token( ping->token, message->piong->hash );
+  gossip->sign_fn( gossip->sign_ctx, message->piong->hash, 32UL, message->piong->signature );
+
+  fd_ping_tracker_track( gossip->ping_tracker,


No need to call track here, we are just responding to someone else. Also don't do the memcpys, just serialize directly into buffer.

mmcgee-jump · 2025-05-19T01:32:07Z

src/flamenco/gossip/fd_gossip_msg.h

+#include "fd_crds_value.h"
+
+
+#define FD_GOSSIP_MSG_MTU (1232UL) /* Maximum size of a gossip message */


No need, use FD_NET_MTU

ravyu-jump added 4 commits May 16, 2025 07:33

setup type defs and fn defintions for parsing

b22d9d7

parsers for ping and pong

9841130

progress

dfafa9b

rx ping/pong, tx ping, ping/pong serializers, helper functions, coale…

df53f71

…sce ping and pong into one type

ravyu-jump changed the title ~~WIP: gossip2 branch~~ WIP: changes to merge gossip2 branch May 16, 2025

init keyguard_client

11a23ee

mmcgee-jump reviewed May 18, 2025

View reviewed changes

mmcgee-jump reviewed May 19, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: changes to merge gossip2 branch #5154

WIP: changes to merge gossip2 branch #5154

ravyu-jump commented May 16, 2025

mmcgee-jump May 18, 2025

mmcgee-jump May 18, 2025

mmcgee-jump May 18, 2025

mmcgee-jump May 18, 2025

mmcgee-jump May 18, 2025

mmcgee-jump May 18, 2025

mmcgee-jump May 18, 2025

mmcgee-jump May 18, 2025

mmcgee-jump May 18, 2025

mmcgee-jump May 18, 2025

mmcgee-jump May 18, 2025

mmcgee-jump May 18, 2025

mmcgee-jump May 18, 2025

mmcgee-jump May 18, 2025

mmcgee-jump May 18, 2025

mmcgee-jump May 18, 2025

mmcgee-jump May 18, 2025

mmcgee-jump May 18, 2025

mmcgee-jump May 18, 2025

mmcgee-jump May 18, 2025

mmcgee-jump May 18, 2025

mmcgee-jump May 18, 2025

mmcgee-jump May 19, 2025

	fd_topo_link_t * sign_out = &topo->links[ tile->out_link_id[ sign_out_tile_idx ] ];
	fd_topo_link_t * sign_out = &topo->links[ tile->out_link_id[ ctx->sign_out->idx ] ];

	err = fd_ed25519_verify( payload + message->signable_data_offset,
	err = fd_ed25519_verify( payload+message->signable_data_offset,

	if( FD_UNLIKELY( err != FD_ED25519_SUCCESS ) ) return err;
	if( FD_UNLIKELY( err!=FD_ED25519_SUCCESS ) ) return err;

	if( FD_UNLIKELY( now-500L1000L1000L>(long)prune->wallclock ) ) return FD_GOSSIP_RX_PRUNE_ERR_TIMEOUT;
	FD_GOSSIP_RX_PRUNE_ERR_STALE;

	uchar * expected_pong_token,
	uchar expected_pong_token[ static 32UL ],

	CHECK( tag<FD_GOSSIP_MESSAGE_END );
	CHECK( tag<=FD_GOSSIP_MESSAGE_LAST );

	struct fd_gossip_contact_info {
	struct fd_gossip_message_contact_info {

	fd_ping_tracker_hash_ping_token( uchar const * token,
	fd_ping_tracker_respond( uchar const * token,

		#include "fd_crds_value.h"


		#define FD_GOSSIP_MSG_MTU (1232UL) /* Maximum size of a gossip message */

WIP: changes to merge gossip2 branch #5154

Are you sure you want to change the base?

WIP: changes to merge gossip2 branch #5154

Conversation

ravyu-jump commented May 16, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment