Skip to content

feat(lazer) Add response messages for lazer transactions #2743

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.

Sign up for GitHub

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
+60 −15
Draft

feat(lazer) Add response messages for lazer transactions #2743

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 13 additions & 13 deletions lazer/Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 23 additions & 0 deletions lazer/publisher_sdk/proto/pyth_lazer_transaction.proto
View file
Open in desktop
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why is this payload so generic anyway? Couldn't we constrain it at all?

pyth-crosschain/lazer/publisher_sdk/proto/pyth_lazer_transaction.proto

Lines 20 to 30 in 518aa8d

// If the signature data is a Ed25519SignatureData, the payload is the encoded
// LazerTransaction protobuf message.
//
// If the signature data is a WormholeMultiSigData, the payload is the encoded
// Wormhole VAA body. The Wormhole VAA can be any of the following:
// 1. A governance message from Pyth that updates Lazer state (e.g. a new feed) which
// is an ecoded GovernancePayload according to xc-admin spec which contains the
// encoded GovernanceInstruction protobuf message.
// 2. A governance message from Wormhole that updates Wormhole guardian set which follows
// the Wormhole specification.
optional bytes payload = 2;

Copy link
Contributor

Choose a reason for hiding this comment

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

It has to be bytes because we need to decode it as bytes to verify the signature.

Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,26 @@ message LazerTransaction {
GovernanceInstruction governance_instruction = 2;
}
}

// Response to SignedLazerTransactionResponse from the relayer
message SignedLazerTransactionResponse {
Copy link
Contributor Author

@bplatak bplatak May 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

💬 We will need to expand this a lot but I haven't figured out the best way to do this yet. The difficult part is handling all of the sub-type of what SignedLazerTransaction can contain (atm it's just an arbitrary binary payload which in effect is always LazerTransaction).

Somehow we should represent PublisherUpdateResponse which could be accept (full or partial) and reject, both possibly having some more contextual details (like what we have for the envelope contexts used internally)

pyth-crosschain/lazer/publisher_sdk/proto/transaction_envelope.proto

Line 38 in 157c756

repeated FeedUpdateContext feed_update_context = 2;

We'll need the same sort of thing for GovernanceInstructionResponse.

We could define the top-level accept message using something like this (and the same for the rejections):

  message SignedLazerTransactionAccepted {
      oneof details {
          PublisherUpdateResponseAccepted publisher_update_accepted = 1;
          GovernanceInstructionResponseAccepted governance_instruction_accepted = 2;
      }
  }

and include further details in those specific messages. We could also include a bytes details field in the message that contains type-specific response (in the same way we do with the payload) but I'm ideologically opposed to that idea.

I think SignedLazerTransactionResponse and LazerTransactionResponse are related but ultimately separate concepts and we need to deal with that.

@pyth-network/price-feeds-team i'd like to hear your ideas, comments, concerns. Let's make sure we're on the same page now because changing this later will be a big pain.

Copy link
Contributor

Choose a reason for hiding this comment

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

In the API endpoints we just want to return JSON, so I'm not sure if there is any reason to define this type in protobuf. It can be just another type definition in our API. As for the structure, we could use a enum with payload for governance/publisher variants, but we can also just dump all possible response details as optional fields on the top object because the sender already knows what type of transaction it sends.

// [required] The ID of a transaction derived as sha256 of the payload.
optional bytes transaction_id = 1;

// [required] Status of the transaction received by the relayer
oneof status {
// The transaction was accepted by the relayer
SignedLazerTransactionAccepted accepted = 2;
// The transaction was rejected by the relayer
SignedLazerTransactionRejected rejected = 3;
}

// Message containing additional details for accepted transactions
message SignedLazerTransactionAccepted {}

// Message containing details why the transaction was rejected
message SignedLazerTransactionRejected {
// [required] Human readable error message
optional string message = 1;
}
}
2 changes: 1 addition & 1 deletion lazer/publisher_sdk/rust/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ pyth-lazer-protocol = { version = "0.7.2", path = "../../sdk/rust/protocol" }
anyhow = "1.0.98"
protobuf = "3.7.2"
serde-value = "0.7.0"
sha2 = "0.10.9"
humantime = "2.2.0"
tracing = "0.1.41"

[build-dependencies]
fs-err = "3.1.0"
Expand Down
24 changes: 23 additions & 1 deletion lazer/publisher_sdk/rust/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
use std::{collections::BTreeMap, time::Duration};

use ::protobuf::MessageField;
use crate::transaction::SignedLazerTransaction;
use ::protobuf::{Message, MessageField};
use anyhow::{bail, ensure, Context};
use humantime::format_duration;
use protobuf::dynamic_value::{dynamic_value, DynamicValue};
use pyth_lazer_protocol::router::TimestampUs;
use sha2::{Digest, Sha256};

pub mod transaction_envelope {
pub use crate::protobuf::transaction_envelope::*;
Expand Down Expand Up @@ -160,3 +162,23 @@ impl TryFrom<DynamicValue> for serde_value::Value {
}
}
}

impl SignedLazerTransaction {
// The id of a SignedLazerTransaction is calculated as sha256 of its entire payload. This is
// an unstable ID (sensitive to any proto changes) are should only be used to correlate
// immediate responses sent from the relayer
pub fn id(&self) -> anyhow::Result<[u8; 32]> {
let mut hasher = Sha256::new();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

❔ Is this actually a good idea? I like the simplicity but this creates some problems (e.g. two separate transactions sent at different times could have the same id which is only OK if all the transactions are idempotent). I think we should consider attaching a UUID on the client side - it's a tiny amount of data and IMHO a more resilient way to identify messages.

Copy link
Contributor

Choose a reason for hiding this comment

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

For governance we have sequence numbers, and for publishers we have publisher timestamps that have to be increasing. Both can be interpreted as a sort of nonce. Therefore, payloads will always be different unless it's literally the same transaction (in which case the second occurrence will be ignored).

self.write_to_writer(&mut hasher)?;
Copy link
Contributor

Choose a reason for hiding this comment

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

This will hash both the signature and payload. We need it to be only payload.

hasher
.finalize()
.try_into()
Copy link
Contributor

Choose a reason for hiding this comment

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

Hashing is infallible. handler.finalize().into() into the array type will work.

.context("failed to calculate the sha256 as transaction id")
}

pub fn calculate_id_from_bytes(payload: &Vec<u8>) -> anyhow::Result<[u8; 32]> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
pub fn calculate_id_from_bytes(payload: &Vec<u8>) -> anyhow::Result<[u8; 32]> {
pub fn id_from_payload(payload: &[u8]) -> [u8; 32] {

Ok(Sha256::digest(payload)
.try_into()
.context("failed to calculate the sha256 as transaction id")?)
}
}
Loading