Support receiving verified transactions from the vortexor by lijunwangs · Pull Request #5321 · anza-xyz/agave

lijunwangs · 2025-03-17T01:26:33Z

Problem

This support receiving verified transactions from the vortexor.

Summary of Changes

The additional argument in the validator can start service receiving the verified transactions:

e.g
:
--tpu-vortexor-receiver-address 10.138.0.136:8100

And the validator can use existing TPU customize address to point its TPU addresses to the vortexor:

e.g.
--public-tpu-address 10.138.0.136:9194 --public-tpu-forwards-address 10.138.0.136:9195

Fixes #

codecov-commenter · 2025-04-09T20:13:34Z

Codecov Report

❌ Patch coverage is 32.11382% with 167 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.9%. Comparing base (36b3586) to head (59b8dca).
⚠️ Report is 3608 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff            @@
##           master    #5321     +/-   ##
=========================================
- Coverage    83.0%    82.9%   -0.1%     
=========================================
  Files         828      830      +2     
  Lines      375858   376142    +284     
=========================================
+ Hits       312113   312182     +69     
- Misses      63745    63960    +215

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

apfitzge · 2025-04-10T14:07:44Z


        let (forward_stage_sender, forward_stage_receiver) = bounded(1024);
-        let sigverify_stage = {
+        let sig_verifier = if let Some(vortexor_receivers) = vortexor_receivers {


I think we should optionally spawn vortexor receiver, but we should still spawn the local sigverify stage.

AFAICT the quic and fetch stages still exist, and even if the ports are not advertised on gossip it's possible to still send to them.

This also makes upgrades tied together. I can't restart my vortexor instance because my validator is relying on it.
We should let the operator switch their advertised tpu port(s) at runtime, so they can switch back to local mode to upgrade vortexor without screwing up the validator.
Or switch to another vortexor instance.

Great point. We plan to support the dynamic management of subscriptions via Admin RPC on the validator. See https://github.com/anza-xyz/agave/blob/master/vortexor/Readme.md

Right - but we shouldn't just let the quic & fetch stages send to a SV that doesn't exist. It's likely we will cause a panic in that case soon, and then it's a vulnerability if I can guess the ports your tpu is on.

Even if we can't switch off of it in this PR, we shouldn't remove SV imo

Good point! Will address

Disable TPU streamers when vortexor receiver is configured.

Sorry for the delay, but this seems the wrong direction to me.
If the vortexor goes down, the operator is forced to restart their node to go back to normal TPU. This seems like a necessary feature to me, not something that we should do as follow-up.

I'm happy to hear other's opinions, as maybe I'm being overly cautious about this.

This feature is only enabled when tpu-vortexor-receiver-address enabled. Auto fullback and heartbeat feature will be delivered in follow-on PRs.

mergify · 2025-04-12T00:23:41Z

⚠️ The sha of the head commit of this PR conflicts with #5775. Mergify cannot evaluate rules on this PR. ⚠️

apfitzge · 2025-04-16T21:48:46Z

+    ($sender:expr, $batch:expr, $count:expr) => {
+        match $sender.send($batch) {
+            Ok(_) => {
+                trace!("Sent batch: {} received from vortexor successfully", $count);


will we ever turn this on? this seems excessive.

It was only used for debugging. It will be enhanced to metrics.

apfitzge · 2025-04-16T21:52:04Z

+                Self::recv_send(
+                    batch_receiver,
+                    recv_timeout,
+                    8,


extract this to a constant. can you comment where it came from?

will do. It is rough estimate to limit the memory usage to max PACKETS_PER_BATCH*8 = 512 PACKETS in the total batches.

apfitzge · 2025-04-16T21:54:11Z

+            ))
        };

        let vote_sigverify_stage = {


Votes do not go through vortexor? will they in the future? if I want to offload the sigverify task it seems it should be fully offloaded.

Agreed. I think we can offload votes to vortexor as well. To be done in future PRs.

…ddress argument by default

apfitzge

Approach seems fine as an intermediate step. We can spawn quic &fetch & sigverifier threads on detecting shutdown or admin rpc change in future PRs.

LGTM - @lijunwangs please get approval from a second relevant person. These are larger arch changes.

lijunwangs · 2025-04-18T16:52:11Z

Approach seems fine as an intermediate step. We can spawn quic &fetch & sigverifier threads on detecting shutdown or admin rpc change in future PRs.

LGTM - @lijunwangs please get approval from a second relevant person. These are larger arch changes.

@sakridge @bw-solana can you help review these as well? Thanks!

sakridge

looks good to me

)" (#6525) The reverted commit introduced the solana-vortexor-receiver crate which was used by solana-core. solana-vortexor-receiver was not set to publish which means solana-core cannot be published either. So, back out solana-vortexor in order to unblock crate publishing This reverts commit e44c17d

)" (#6525) The reverted commit introduced the solana-vortexor-receiver crate which was used by solana-core. solana-vortexor-receiver was not set to publish which means solana-core cannot be published either. So, back out solana-vortexor in order to unblock crate publishing This reverts commit e44c17d (cherry picked from commit 63cf093) # Conflicts: # Cargo.lock # Cargo.toml # programs/sbf/Cargo.lock # svm/examples/Cargo.lock # vortexor-receiver/Cargo.toml

)" (#6525) The reverted commit introduced the solana-vortexor-receiver crate which was used by solana-core. solana-vortexor-receiver was not set to publish which means solana-core cannot be published either. So, back out solana-vortexor in order to unblock crate publishing This reverts commit e44c17d (cherry picked from commit 63cf093)

…or (#5321)" (backport of #6525) (#6529) The reverted commit introduced the solana-vortexor-receiver crate which was used by solana-core. solana-vortexor-receiver was not set to publish which means solana-core cannot be published either. So, back out solana-vortexor in order to unblock crate publishing This reverts commit e44c17d (cherry picked from commit 63cf093) --------- Co-authored-by: steviez <steven@anza.xyz>

#6542) * Revert "Revert "Support receiving verified transactions from the vortexor (#5321)" (#6525)" This reverts commit 63cf093. * publish vortexor-receiver as it is used by core * Rename solana-vortexor-receiver to agave-verified-packet-receiver * Missed Cargo.lock update

lijunwangs force-pushed the vortexor_verified_packets_receiver branch from 70dcb08 to bf932aa Compare April 9, 2025 19:22

lijunwangs commented Apr 9, 2025

View reviewed changes

Comment thread vortexor-receiver/Readme.md Outdated

lijunwangs requested review from apfitzge, bw-solana, pgarg66 and sakridge April 10, 2025 03:01

lijunwangs force-pushed the vortexor_verified_packets_receiver branch from 461cfc7 to ab02dd8 Compare April 10, 2025 04:14

lijunwangs requested a review from jstarry April 10, 2025 04:31

apfitzge reviewed Apr 10, 2025

View reviewed changes

Comment thread core/src/vortexor_receiver_adapter.rs Outdated

lijunwangs requested a review from apfitzge April 14, 2025 17:07

lijunwangs force-pushed the vortexor_verified_packets_receiver branch from d956aae to e86c96f Compare April 15, 2025 18:01

apfitzge requested review from apfitzge and removed request for apfitzge April 15, 2025 19:27

apfitzge reviewed Apr 16, 2025

View reviewed changes

lijunwangs mentioned this pull request Apr 17, 2025

Votes do not go through vortexor? will they in the future? if I want to offload the sigverify task it seems it should be fully offloaded. #5864

Open

lijunwangs added 9 commits April 17, 2025 02:37

Support receiving verified transactions from the vortexor

73965a3

vortexor receiver adapter

8061476

Update some comments

3fd83f0

Update some comments

053ae73

Gaurd against 0 packets

08c8e6b

update some debug messages

9c8b817

disable regular tpu streamers if vortexor is in-place

bfb49f3

disable regular tpu streamers if vortexor is in-place

bb8280e

Addressed some comments from Andrew. Hide the tpu-vortexor-receiver-a…

59b8dca

…ddress argument by default

lijunwangs force-pushed the vortexor_verified_packets_receiver branch from fbe8b06 to 59b8dca Compare April 17, 2025 09:45

lijunwangs requested a review from apfitzge April 17, 2025 16:59

apfitzge reviewed Apr 17, 2025

View reviewed changes

sakridge approved these changes Apr 21, 2025

View reviewed changes

lijunwangs merged commit e44c17d into anza-xyz:master Apr 21, 2025
41 checks passed

steviez mentioned this pull request Jun 12, 2025

Revert "Support receiving verified transactions from the vortexor (#5321)" #6525

Merged

mergify Bot mentioned this pull request Jun 12, 2025

v2.3: Revert "Support receiving verified transactions from the vortexor (#5321)" (backport of #6525) #6529

Merged

lijunwangs mentioned this pull request Jun 18, 2025

Restore vortexor receiver -- renamed to agave-verified-packet-receiver #6542

Merged

Conversation

lijunwangs commented Mar 17, 2025

Problem

Summary of Changes

Uh oh!

Uh oh!

codecov-commenter commented Apr 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

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!

lijunwangs Apr 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mergify Bot commented Apr 12, 2025

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

apfitzge left a comment

Choose a reason for hiding this comment

Uh oh!

lijunwangs commented Apr 18, 2025

Uh oh!

sakridge left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov-commenter commented Apr 9, 2025 •

edited

Loading

lijunwangs Apr 16, 2025 •

edited

Loading