Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hook service #2

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Hook service #2

wants to merge 4 commits into from

Conversation

buffalu
Copy link
Owner

@buffalu buffalu commented Jan 31, 2024

Problem

Summary of Changes

Fixes #

uint32 port = 2;
}

message EnableTpuRequest {

Choose a reason for hiding this comment

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

is this referring to validator's local tpu?

bool enable_tpu_fwd = 2;
}

message EnableTpuResponse {}
Copy link

@segfaultdoc segfaultdoc Jan 31, 2024

Choose a reason for hiding this comment

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

return tpu sockets?

}

message SendPacketsRequest {
bool admin = 1;

Choose a reason for hiding this comment

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

can you leave a comment on what this is?

@@ -0,0 +1,93 @@
syntax = "proto3";

Choose a reason for hiding this comment

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

is this generally an admin hook service? if so can you leave a comment on that


message SetShredForwarderAddressResponse {}

message SetMaxComputeRequest {

Choose a reason for hiding this comment

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

can you rename to something that's obvious this is related to bundle CU reserve

message SetMaxComputeResponse {}

message Bundle {
bool admin = 1;

Choose a reason for hiding this comment

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

what does this denote?

message Bundle {
bool admin = 1;
repeated Packet packets = 2;
string uuid = 3;

Choose a reason for hiding this comment

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

bundle ids are not UUIDs anymore, rename to id

// Sends packets into the BankingStage of the validator
rpc SendPackets(stream SendPacketsRequest) returns (SendPacketsResponse);

// Marks accounts for discard

Choose a reason for hiding this comment

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

can you elaborate on this method?

rpc MarkAccountsForDiscard(MarkAccountsForDiscardRequest) returns (MarkAccountsForDiscardResponse);

// Sets the shred forwarder addresses
rpc SetShredForwarderAddress(SetShredForwarderAddressRequest) returns (SetShredForwarderAddressResponse);

Choose a reason for hiding this comment

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

could also consider having broadcast stream packets back to sidecar and have that manage shred forwards. down side is if side car crashes so does shred forward

bool enable_tpu_fwd = 2;
}

message EnableTpuResponse {}

Choose a reason for hiding this comment

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

should we return the old values?

Choose a reason for hiding this comment

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

ditto on most of the other SetXXX methods

}

message SendPacketsRequest {
bool admin = 1;

Choose a reason for hiding this comment

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

what is the admin flag for?

message SetMaxComputeResponse {}

message Bundle {
bool admin = 1;

Choose a reason for hiding this comment

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

ditto


message SendBundleRequest {
repeated Bundle bundles = 1;
uint64 slot_expiration = 2;

Choose a reason for hiding this comment

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

is this inclusive or exclusive? eg if slot = 5, do we still consider the bundle on slot 5?

Choose a reason for hiding this comment

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

comment would be good

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.

3 participants