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

Remove dependency on originator node for blockchain envelopes #224

Merged
merged 3 commits into from
Oct 18, 2024

Conversation

richardhuaaa
Copy link
Contributor

We allow anyone to publish to the blockchain directly, so there is no need to specify the publisher_node_id. The originator_ns field on the UnsignedOriginatorEnvelope will be derived from the block time.

@richardhuaaa richardhuaaa requested a review from a team as a code owner October 18, 2024 18:55
@@ -53,7 +52,6 @@ message UnsignedOriginatorEnvelope {
// An alternative to a signature for blockchain payloads
message BlockchainProof {
uint64 block_number = 1;
uint32 publisher_node_id = 2;
Copy link
Collaborator

Choose a reason for hiding this comment

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

We could also include the transaction hash, since this proof gets generated when the message is read off the chain. That would make it easier to verify

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we expect recipients to verify both the block_number and transaction_hash? Or would the transaction_hash alone be sufficient to find the proof? Just trying to avoid redundant fields that need to be kept consistent where possible

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(Also, you can merge first, I'll apply this on top of your change)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Think the hash could replace the block number, since you can get the block number with the hash

@richardhuaaa richardhuaaa enabled auto-merge (squash) October 18, 2024 21:44
@richardhuaaa richardhuaaa merged commit 3b52250 into main Oct 18, 2024
4 checks passed
@richardhuaaa richardhuaaa deleted the rich/remove-originator-dependency branch October 18, 2024 21:45
Copy link

🎉 This PR is included in version 3.72.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants