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

Add off-chain messages support #27456

Merged
merged 3 commits into from
Oct 17, 2022
Merged

Conversation

askibin
Copy link
Contributor

@askibin askibin commented Aug 29, 2022

Problem

#21366

Summary of Changes

Adds signing of non-transaction messages with a Solana wallet. This feature can be used to authenticate users or provide proof of wallet ownership.
See Docs for more details.
In accordance with the Proposal.
Off-chain messages can be signed and verfied using Solana CLI and any of the supported wallets including Ledger. Latest firmware and Solana Ledger App v1.3.0 or higher is required for this feature to work.

Fixes #

@askibin askibin force-pushed the offchain-message branch 7 times, most recently from 08f123d to aa52c1c Compare September 30, 2022 19:47
@askibin askibin changed the title Draft: Add off-chain messages support Add off-chain messages support Sep 30, 2022
@askibin askibin marked this pull request as ready for review September 30, 2022 19:59
@askibin askibin force-pushed the offchain-message branch 2 times, most recently from edc8903 to 91473a9 Compare October 3, 2022 14:30
@t-nelson
Copy link
Contributor

t-nelson commented Oct 7, 2022

just realizing i haven't finished addressing the final open issues in the spec. i'll try to wrap that up today

@CriesofCarrots
Copy link
Contributor

@t-nelson @askibin , can one of you please link to the spec here?

@askibin
Copy link
Contributor Author

askibin commented Oct 7, 2022

@t-nelson @askibin , can one of you please link to the spec here?

description has the link to the proposal

remote-wallet/src/ledger.rs Show resolved Hide resolved
remote-wallet/src/ledger.rs Outdated Show resolved Hide resolved
docs/src/cli/sign-offchain-message.md Outdated Show resolved Hide resolved
docs/src/cli/sign-offchain-message.md Show resolved Hide resolved
sdk/src/offchain_message.rs Outdated Show resolved Hide resolved
sdk/src/offchain_message.rs Outdated Show resolved Hide resolved
sdk/src/offchain_message.rs Outdated Show resolved Hide resolved
sdk/src/offchain_message.rs Outdated Show resolved Hide resolved
cli/src/wallet.rs Outdated Show resolved Hide resolved
cli/src/wallet.rs Outdated Show resolved Hide resolved
@askibin askibin force-pushed the offchain-message branch 2 times, most recently from 53fbd75 to 2d8bbef Compare October 11, 2022 20:17
Copy link
Contributor

@CriesofCarrots CriesofCarrots left a comment

Choose a reason for hiding this comment

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

Thanks for all the changes! I have a couple more thoughts, but getting close I think.

sdk/src/offchain_message.rs Outdated Show resolved Hide resolved
sdk/src/offchain_message.rs Outdated Show resolved Hide resolved
sdk/src/offchain_message.rs Outdated Show resolved Hide resolved
sdk/src/offchain_message.rs Outdated Show resolved Hide resolved
impl OffchainMessage {
pub const SIGNING_DOMAIN: &'static [u8] = b"\xffsolana offchain";
// Header Length = Signing Domain (16) + Header Version (1)
pub const HEADER_LEN: usize = Self::SIGNING_DOMAIN.len() + 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

const_assert_eq! here?

docs/src/cli/sign-offchain-message.md Outdated Show resolved Hide resolved
docs/src/cli/sign-offchain-message.md Outdated Show resolved Hide resolved
sdk/src/offchain_message.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@CriesofCarrots CriesofCarrots left a comment

Choose a reason for hiding this comment

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

One super nitty nit. Otherwise, that's all from me! Thanks for the improvements on this

sdk/src/offchain_message.rs Show resolved Hide resolved
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