solana-labs / solana

Web-Scale Blockchain for fast, secure, scalable, decentralized apps and marketplaces.
https://solanalabs.com
Apache License 2.0
13.1k stars 4.24k forks source link

Add the ability to sign a message to the Solana Ledger app #21366

Closed fragosti closed 1 year ago

fragosti commented 2 years ago

Problem

At Phantom, we've implemented a signMessage feature that allows an application to request signing an arbitrary message. This feature is being used by Audius on Solana to authenticate users, and is popular on Ethereum for applications like OpenSea that also rely on it to authenticate their users.

A long-standing issue with this feature is that the Solana Ledger app (https://github.com/LedgerHQ/app-solana) does not support signing arbitrary messages, and so this feature does not work for Ledger users.

Proposed Solution

Add support for signing arbitrary data (or just utf8 strings) to the Solana Ledger app. @t-nelson mentioned that support would need to be added to solana-remote-wallet and the CLI as well.

t-nelson commented 2 years ago

For all the people requesting this for monkeyleague, ask your developers to seek for an alternative to this, take a look at implementations from other projects like pesky penguins, yawww, matrica, grape, etc.

To sign arbitrary data with a ledger is not a priority for Solana core rn.

every post in this vein demotes it in my priority queue! keep it up! you brought two tag-alongs with you gg.

yes, of course off chain message signing is prioritized below relieving congestion. get over yourselves

Anas11180 commented 2 years ago

would love to have a fix for sites like: https://app.monkeyleague.io/store ❤️

zkRemda commented 2 years ago

For all the people requesting this for monkeyleague, ask your developers to seek for an alternative to this, take a look at implementations from other projects like pesky penguins, yawww, matrica, grape, etc.

To sign arbitrary data with a ledger is not a priority for Solana core rn.

every post in this vein demotes it in my priority queue! keep it up! you brought two tag-alongs with you gg.

yes, of course off chain message signing is prioritized below relieving congestion. get over yourselves

Seems like you miss understood my message, nvm

safaiyeh commented 2 years ago

This is something we need at Magic Eden as well. Our profile login is dependent on a message signing

0xMQQ commented 2 years ago

o sign arbitrary data with a ledger is not a priority for Solana co

Ridiculous! Not much more I can say about this comment.

Totally agree. So many major web3 apps in Solana, including some of the biggest NFT marketplaces, depend on this functionality right now. It sounds so arrogant to say that improving user security and experience is not a priority.

tribixbite commented 2 years ago

We need this for https://claim-map.metaversol.com. As long as we are all sharing lol.

Anas11180 commented 2 years ago

would love to have a fix for sites like: https://app.monkeyleague.io/store

NickTikhonov commented 2 years ago

This fix is crucial - it blocks adoption of good security practices for Solana end-users. cc @vjohnston

Relevant product context: Signing arbitrary data using a wallet is necessary for creating progressively decentralised applications. These are applications that experiment with features in web2, before implementing them on-chain. They rely on a mixture of both web3 and web2 (API backend) interactions. Many of the companies that are building products this way have stated that this is a blocker for them ^

Concrete use case for this feature: One common use case is creating a separate user account, stored in a database tied to a specific wallet owner. This pattern is incredibly common - as it allows the storage of data off-chain, using centralised services - but still have that data be secured by the private key of a wallet holder. For example, this can be used to store rich information for user profiles - which would otherwise be very expensive to store on-chain!

Specific technical issue that blocks the use case: This example requires the user to sign a message and send that to an API backend - which verifies that the sender is indeed the holder of the private key (and hence wallet). Similarly any write operation to an API, that is tied to a user account, requires further signatures - if the application chooses not to implement a bespoke authentication flow (many choose not to).

Impact on Solana end-users: Currently, keeping this feature as low-priority prevents many high-quality web3 user experiences from being available to Ledger users. For example, in our application (https://app.littleatlas.xyz) - you can't use any features aside from minting (for example, creating custom profiles and visual status updates) if you are a Ledger user.

Using a software wallet like Phantom and compromising security is the only way for our users to have the optimal user experience when using web3 applications on Solana.

Hence, unless there is a good workaround for Ledger users that we aren't aware of, we'd argue that reprioritising this should be considered!

kizzx2 commented 2 years ago

Since it isn't like this is coming any time soon, I've created an app that let's user just send a amount of SOL to "sign a message": https://github.com/kizzx2/solana-sign-with-payment

queiroz commented 2 years ago

Me as a Ledger loyal customer and collecting NFT's and going over many Dapps, they all seem to rely on message signing and yet I am forced to move assets off Ledger. This should be prioritised as many Ledger users in the Solana eco-system are moving away because they have no other alternative.

josezy commented 2 years ago

We need to sign messages in Cyber Pharmacy too: https://cyberpharmacy.io/staking/stake 💯

Plumillon commented 2 years ago

Still no update on this one? Many projects are blocked by this and end up charged a tiny amount to bypass (ugly workaround).

alexgidge commented 2 years ago

Any updates? This is required for authentication using Ledger. Every Solana Web3 App should need for authentication

CryptoGod69420 commented 2 years ago

Magic Eden wallet drained last week. Can you guess why?

Oh ya it's because in order to get the magic ticket airdrop you were literally forced off your ledger.

End users keep this space alive at the EOD, whatever we deem necessary should rest at top of dev to-do list

Been in Sol long time, I've personally waited over a year and a half for this capability, how much longer must we wait, and how much more must we lose along the way

quarantaquatro commented 2 years ago

Magic Eden wallet drained last week. Can you guess why?

Oh ya it's because in order to get the magic ticket airdrop you were literally forced off your ledger.

End users keep this space alive at the EOD, whatever we deem necessary should rest at top of dev to-do list

Been in Sol long time, I've personally waited over a year and a half for this capability, how much longer must we wait, and how much more must we lose along the way

Don't expect any reply from them here, they do not care much about this subject, unfortunately!

C-o-d-e-C-o-w-b-o-y commented 2 years ago

Please

levicook commented 2 years ago

Uh, wow. Showed up to this thread ready to contribute now feel like that would be unwelcome and a waste of time. gl everyone.

ChewingGlass commented 2 years ago

Not that it's particularly helpful, but both Strata Protocol and Lit Protocol need this. What is the holdup? Any way I can help?

solserer-labs commented 2 years ago

How is signing messages with a hardware wallet NOT a priority? Tbh thought we had this forever.

swolswolomon commented 2 years ago

Add Cyber Samurai to the list of collections that really want this! Thanks!

21e8 commented 2 years ago

ok now lets assemble a list of people who are qualified to do this, we all agree its needed. and trent seems busy.

JonCognioDigital commented 2 years ago

every post in this vein demotes it in my priority queue!

I had no idea the Solana devs were 8 years old.

Seriously, this is BASIC functionality and necessary for lots of sites to function properly. The fact that this ticket is still open is embarrassing and saying it's a feature not a bug?

"Hey, my car doesn't have an engine, but that's a feature not a bug, I like it that way. Oh, and BTW it never had one so it can't be broken!"

zkRemda commented 2 years ago

every post in this vein demotes it in my priority queue!

I had no idea the Solana devs were 8 years old.

Seriously, this is BASIC functionality and necessary for lots of sites to function properly. The fact that this ticket is still open is embarrassing and saying it's a feature not a bug?

"Hey, my car doesn't have an engine, but that's a feature not a bug, I like it that way. Oh, and BTW it never had one so it can't be broken!"

Hi, just to add some context. To sign arbitrary messages is not a solana feature. If you read the first message in this thread, is explained it is a feature implemented by Phantom but is not a core functionality in Solana.

Remember the priority for Solana devs is Solana core, not a side functionality implemented by a third party.

The whole point of a blockchain is to have On-chain transactions, to login with an arbitrary message is a web2 experience. So go back to your communities and design a web3 solution for it rather than complaining about solana not prioritizing things that aren't important for the architecture.

GrimLothar commented 2 years ago

every post in this vein demotes it in my priority queue!

I had no idea the Solana devs were 8 years old. Seriously, this is BASIC functionality and necessary for lots of sites to function properly. The fact that this ticket is still open is embarrassing and saying it's a feature not a bug? "Hey, my car doesn't have an engine, but that's a feature not a bug, I like it that way. Oh, and BTW it never had one so it can't be broken!"

Hi, just to add some context. To sign arbitrary messages is not a solana feature. If you read the first message in this thread, is explained it is a feature implemented by Phantom but is not a core functionality in Solana.

Remember the priority for Solana devs is Solana core, not a side functionality implemented by a third party.

The whole point of a blockchain is to have On-chain transactions, to login with an arbitrary message is a web2 experience. So go back to your communities and design a web3 solution for it rather than complaining about solana not prioritizing things that aren't important for the architecture.

So if it's not important for Solana, why is Anatoly supporting that this is implemented? https://twitter.com/aeyakovenko/status/1547634308646481923

I don't understand the push back on this issue (whether it is an issue or a feature request). It is clear it is wanted by a LOT of communities. Implementing this is a net positive for Solana as a whole.

zkRemda commented 2 years ago

So if it's not important for Solana, why is Anatoly supporting that this is implemented? https://twitter.com/aeyakovenko/status/1547634308646481923

It would happen... eventually. Just chill and wait. 😉

GrimLothar commented 2 years ago

So if it's not important for Solana, why is Anatoly supporting that this is implemented? https://twitter.com/aeyakovenko/status/1547634308646481923

It would happen... eventually. Just chill and wait. 😉

Oh I'm chilling and waiting since January. No complaints from me on how long it takes to implement this. Even implemented a non ideal workaround until it happens. I'm just commenting on the pushback itself...

0xMQQ commented 2 years ago

every post in this vein demotes it in my priority queue!

I had no idea the Solana devs were 8 years old. Seriously, this is BASIC functionality and necessary for lots of sites to function properly. The fact that this ticket is still open is embarrassing and saying it's a feature not a bug? "Hey, my car doesn't have an engine, but that's a feature not a bug, I like it that way. Oh, and BTW it never had one so it can't be broken!"

Hi, just to add some context. To sign arbitrary messages is not a solana feature. If you read the first message in this thread, is explained it is a feature implemented by Phantom but is not a core functionality in Solana.

Remember the priority for Solana devs is Solana core, not a side functionality implemented by a third party.

The whole point of a blockchain is to have On-chain transactions, to login with an arbitrary message is a web2 experience. So go back to your communities and design a web3 solution for it rather than complaining about solana not prioritizing things that aren't important for the architecture.

I am sorry to say that, but this is exactly what I meant "arrogant". You make it sound like everyone in this thread is just dumbass who doesn't understand web3 and blockchain. I don't know how this is helpful.

I would suggest you read the first message again. Phantom, the most widely used Solana wallet, one of the most important piece in the Solana ecosystem, came to the core team for help! and you tell them to "go back"??

I have searched around about this issue since months ago, and I am fully aware that there are similar open issues in Ledger and Phantom's github repos. This is not a super technical challenging issue, but it has big impact and it does require some coordination between multiple parties. This is why people have come here for help. This is where people expect the core team can take some leadership to bring partners together to resolve it.

Yes, this may not be a direct tech responsibility of the core team. But instead of tell people to "go back to your communities", how about help pass the message up, raise awareness, and find the right contact person that can help coordinate and resolve the issue? Thank you.

josezy commented 2 years ago

I ended up creating a transaction with a memo instruction in it (a nonce) then verifying the signature (and the nonce), not ideal but it worked (tx is never sent to the chain, only signed and verified)

import { PublicKey, Transaction, TransactionInstruction } from "@solana/web3.js"

const MEMO_PROGRAM_ID = new PublicKey(
  "MemoSq4gqABAXKb96qnH8TysNcWxMyWCqXgDLGmfcHr"
)

export const buildAuthTx = (nonce: string): Transaction => {
  const tx = new Transaction()
  tx.add(
    new TransactionInstruction({
      programId: MEMO_PROGRAM_ID,
      keys: [],
      data: Buffer.from(nonce, "utf8"),
    })
  )
  return tx
}

export const validateAuthTx = (tx: Transaction, nonce: string): boolean => {
  try {
    const inx = tx.instructions[0]
    if (!inx.programId.equals(MEMO_PROGRAM_ID)) return false
    if (inx.data.toString() != nonce) return false
    if (!tx.verifySignatures()) return false
  } catch (e) {
    return false
  }
  return true
}

How to use it:

// Create tx
const tx = buildAuthTx("test-nonce")
tx.feePayer = wallet.publicKey // not sure if needed but set this properly
tx.recentBlockhash = (await connection.getLatestBlockhash()).blockhash // same as line above

// Encode and send tx to signer, decode and sign
signedTx = await wallet.signTransaction(tx)

// Encode, send back, decode and verify signedTx signature
const validSignature = validateAuthTx(signedTx, "test-nonce")
JonCognioDigital commented 2 years ago

The whole point of a blockchain is to have On-chain transactions, to login with an arbitrary message is a web2 experience. So go back to your communities and design a web3 solution for it rather than complaining about solana not prioritizing things that aren't important for the architecture.

Wow.

Just.... wow ......

jordaaash commented 2 years ago

I ended up creating a transaction with a memo instruction in it (a nonce) then verifying the signature (and the nonce), not ideal but it worked (tx is never sent to the chain, only signed and verified)

This is a cool solution!

A way you could make it safe for the user to sign without any risk of costing them SOL is to have the memo instruction require a signature from an account public key that you can't have the private key for (like new PublicKey(0)).

This should make it so that the instruction would fail in signature verification if ever submitted, and could never cost them fees. Alternatively, you could just set the fee payer to this (which of course, can't pay fees at all).

Edit: You also don't need the recent blockhash to be valid, since it's never getting sent. Might as well save yourself the RPC call!

aeyakovenko commented 1 year ago

Fixed in https://github.com/solana-labs/solana/pull/27456

GrimLothar commented 1 year ago

Fixed in #27456

It's a Breakpoint miracle! 🙌

Jokes aside, thank you so very much to everyone who worked on this. ❤️

dahifi commented 1 year ago

ZOMG thank you so much.

jnemonic commented 1 year ago

This is UUGE On Nov 2, 2022, 10:00 AM -0700, anatoly yakovenko @.***>, wrote:

Fixed in #27456 — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.Message ID: @.***>

0xMQQ commented 1 year ago

Fixed in #27456

This is AWESOME! Thank you so much for working on this!

kelonye commented 1 year ago

@jordansexton looking forward to seeing this working in the adapter lib..

roguzh commented 1 year ago

could just set the fee payer to this (which of course, can't p

I have been following your suggestions but got confused at the part about requiring signature from an account public key that you can't have the private key.

Would it be possible to verify the actual wallet's (user's) signature?

JonCognioDigital commented 1 year ago

For those of us running a site which uses the Solana Wallet Adapter library supporting Phantom and Solflare, do we need to do anything to implement this? DO we have to wait for Phantom/Solflare/Ledger to add something or do we need to do anything ourselves?

jordaaash commented 1 year ago

@JonCognioDigital yes, this is primarily on Phantom, Solflare, and other wallets to add support for.

It's also important to note that messages signed this way have a specific format, described here: https://github.com/solana-labs/solana/blob/e80f67dd58b7fa3901168055211f346164efa43a/docs/src/proposals/off-chain-message-signing.md

Wallets and apps will both need to update how they handle message signing in accordance with this.

fragosti commented 1 year ago

We will be adding support for this at Phantom :) Thank you for implementing 👍

fakenine commented 1 year ago

Hello! Just installed version 1.4.1 and I've got the same error message cc @fragosti