Skip to content

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

Merged
lijunwangs merged 4 commits intoanza-xyz:masterfrom
lijunwangs:revert_revert_vortexor_receiver
Jun 25, 2025
Merged

Restore vortexor receiver -- renamed to agave-verified-packet-receiver#6542
lijunwangs merged 4 commits intoanza-xyz:masterfrom
lijunwangs:revert_revert_vortexor_receiver

Conversation

@lijunwangs
Copy link
Copy Markdown

@lijunwangs lijunwangs commented Jun 12, 2025

Problem

restore the vortexor receiver code which was reverted due to unpublished dependency

Summary of Changes

Revert revert vortexor receiver
renamed to agave-verified-packet-receiver
publishing agave-verified-packet-receiver

Fixes #

Notes:

The original PR for this functionality:
#5321

The revert PR which unblocked publishing

#6525

@steviez
Copy link
Copy Markdown

steviez commented Jun 12, 2025

@lijunwangs - I was under the impression vortexor was still a WIP; I can see that solana-vortexor is still set with publish = false. Can you comment on the status of this project ?

Some other general questions:

  • Why solana- instead of agave- ?
  • Wasn't vortexor going to live in a separate repo long term ? Publishing them here and now makes things slightly more concrete in monorepo ?
  • Vortexor receiver looks like a fairly generic component; ie, just some way to listen for pre-sigverified transactions. If this is something that we want to introduce into our code right now, I'm wondering if it should be made more generic

@lijunwangs
Copy link
Copy Markdown
Author

@lijunwangs - I was under the impression vortexor was still a WIP; I can see that solana-vortexor is still set with publish = false. Can you comment on the status of this project ?

Vortexor is now working end-to-end before this was reverted. Of course, we keep improving it by adding features but basically it is working.

Some other general questions:

  • Why solana- instead of agave- ?

I am not sure about the rule. Most of our crate are with solana- prefixed and I keep that. Open to change to agave- if it makes sense.

  • Wasn't vortexor going to live in a separate repo long term ? Publishing them here and now makes things slightly more concrete in monorepo ?

In the future, we do have plan to move the solana-vortexor out. But this PR is about the vortexor receiver which is lib used by the validator. I think the vortexor-receiver belongs this repo.

  • Vortexor receiver looks like a fairly generic component; ie, just some way to listen for pre-sigverified transactions. If this is something that we want to introduce into our code right now, I'm wondering if it should be made more generic

Currently it is very simple just a thin wrapper to start a streamer. In the future we will support optionally using QUIC streamer and authentication validating vortexor identity and etc.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jun 12, 2025

Codecov Report

Attention: Patch coverage is 31.55738% with 167 lines in your changes missing coverage. Please review.

Project coverage is 82.8%. Comparing base (372535d) to head (e3ce7fa).
Report is 48 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##           master    #6542     +/-   ##
=========================================
- Coverage    82.8%    82.8%   -0.1%     
=========================================
  Files         849      851      +2     
  Lines      379408   379689    +281     
=========================================
+ Hits       314277   314416    +139     
- Misses      65131    65273    +142     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@lijunwangs
Copy link
Copy Markdown
Author

the remote server responded with an error (status 400 Bad Request): A crate with the name solana-vortexor-receiver was recently deleted. Reuse of this name will be available after 2025-06-13T16:20:04Z.
Need wait a little longer before it can be published.

@steviez
Copy link
Copy Markdown

steviez commented Jun 17, 2025

I am not sure about the rule. Most of our crate are with solana- prefixed and I keep that. Open to change to agave- if it makes sense.

Gotcha, I guess we don't have a firm rule but I think solana- dates back to older days. For me, I think stuff that is agave specific should use agave and that general stuff (like solana-sdk) can keep solana. I might ask in Discord but probably not a blocker fo rthis PR

In the future, we do have plan to move the solana-vortexor out. But this PR is about the vortexor receiver which is lib used by the validator. I think the vortexor-receiver belongs this repo

Changes to the vortexor receiver should line up with changes to actual vortexor right ? Ie, if the bin supports QUIC then the receiver would need to as well. Are those set to happen in the short term.

And semi-related, is there a timeline for when vortexor moves into its own repo ?

Currently it is very simple just a thin wrapper to start a streamer. In the future we will support optionally using QUIC streamer and authentication validating vortexor identity and etc.

Gotcha, my question was whether the receiver should be considered a "generic" component. Ie, would it be the case that someone potentially writes their own implementation that is comparable to vortexor and wants to use their custom piece ? if so, I'd argue a more generic name might fit. This name is probably too verbose but might help get across what I'm getting at:

agave-tpu-sigverified-transaction-receiver

@lijunwangs
Copy link
Copy Markdown
Author

Gotcha, I guess we don't have a firm rule but I think solana- dates back to older days. For me, I think stuff that is agave specific should use agave and that general stuff (like solana-sdk) can keep solana. I might ask in Discord but probably not a blocker fo rthis PR

I can rename it to agave-verified-packet-receiver.

Changes to the vortexor receiver should line up with changes to actual vortexor right ? Ie, if the bin supports QUIC then the receiver would need to as well. Are those set to happen in the short term.

Yes, under some case, impacting the protocol used between them we need make changes on both sides. The receiver side can also have other changes on its own for improvements. In my opinion it is simpler put the receiver support library in the mono repo.

And semi-related, is there a timeline for when vortexor moves into its own repo ?

I was planning wrapping my current work -- heartbeat support for vortexor. Then work on splitting the vortexor binary off to its own repo.

Gotcha, my question was whether the receiver should be considered a "generic" component. Ie, would it be the case that someone potentially writes their own implementation that is comparable to vortexor and wants to use their custom piece ? if so, I'd argue a more generic name might fit. This name is probably too verbose but might help get across what I'm getting at:

agave-tpu-sigverified-transaction-receiver

It is is related to question number 1, how do you like agave-verified-packet-receiver -- (this is actually more inline with the design doc. https://github.com/anza-xyz/agave/tree/master/vortexor)

@lijunwangs
Copy link
Copy Markdown
Author

@steviez I renamed the crate to agave-verified-packet receiver. Can you follow up on this? Thanks

@apfitzge
Copy link
Copy Markdown

Can we get an accurate description of the PR in the title? This is not just reverting a previously reverted PR.

@lijunwangs lijunwangs changed the title Revert revert vortexor receiver Restore vortexor receiver -- renamed to agave-verified-packet-receiver Jun 18, 2025
@lijunwangs
Copy link
Copy Markdown
Author

Can we get an accurate description of the PR in the title? This is not just reverting a previously reverted PR.

Done

@KirillLykov
Copy link
Copy Markdown

I sort of have lack of context here because I don't know what exactly Vortexor is doing. My understanding is it will be replacement of the jito-relayer but maybe I'm wrong.

@lijunwangs lijunwangs requested a review from sakridge June 18, 2025 19:44
@lijunwangs
Copy link
Copy Markdown
Author

I sort of have lack of context here because I don't know what exactly Vortexor is doing. My understanding is it will be replacement of the jito-relayer but maybe I'm wrong.

Please refer to https://github.com/anza-xyz/agave/tree/master/vortexor for context

Comment thread core/src/tpu.rs
enable_block_production_forwarding.then(|| forward_stage_sender.clone()),
);
SigVerifyStage::new(packet_receiver, verifier, "solSigVerTpu", "tpu-verifier")
SigVerifier::Local(SigVerifyStage::new(
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

To confirm, if we're using vortexor then we don't create a local sigverifier and thus have no way to ingest transactions should vortexor connection get interrupted right ?

I guess vortexor is opt-in as of now so this is possibly fine

Copy link
Copy Markdown
Author

@lijunwangs lijunwangs Jun 24, 2025

Choose a reason for hiding this comment

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

Yes. That is the current behavior. In the heartbeat support which I am going to submit PR for after this, we will monitor traffic and heartbeat from the vortexor. And if we decide the vortexor is down or disconnected, it can auto switch back to the local sig verifier.

@steviez
Copy link
Copy Markdown

steviez commented Jun 20, 2025

Yes, under some case, impacting the protocol used between them we need make changes on both sides. The receiver side can also have other changes on its own for improvements. In my opinion it is simpler put the receiver support library in the mono repo.

Hmm ok, we can revisit down the road if necessary

I was planning wrapping my current work -- heartbeat support for vortexor. Then work on splitting the vortexor binary off to its own repo.

Nice

@lijunwangs lijunwangs requested a review from steviez June 24, 2025 05:56
@steviez
Copy link
Copy Markdown

steviez commented Jun 25, 2025

I'm not positive about either component living in the monorepo in the long term, but given that it is opt-in for the vortexor let's continue and we can revisit when we get closer to cutting v3.0 branch.

@lijunwangs lijunwangs merged commit 18238bd into anza-xyz:master Jun 25, 2025
40 of 41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants