solana-labs / solana

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

Add `--memo` support to solana command-line #10831

Closed mvines closed 3 years ago

mvines commented 4 years ago

With the SPL Memo program on mainnet-beta (http://explorer.solana.com/account/Memo1UhkJRfHyvLMcVucJwxXeuD728EqVDDwQDxFMNo), we can now add a generic --memo argument to all cli commands that issue transactions to include a memo.

bji commented 3 years ago

Hi, I'll take a look at this, if anyone else is working on it or wants to work on it, please let me know!

CriesofCarrots commented 3 years ago

Updated SPL Memo program is here: https://explorer.solana.com/address/MemoSq4gqABAXKb96qnH8TysNcWxMyWCqXgDLGmfcHr SPL Memo source: https://github.com/solana-labs/solana-program-library/tree/master/memo/program Let me know if you have any questions!

bji commented 3 years ago

In your opinion, does the memo program deserve the same level of support as nonce?

I ask because I am still familiarizing myself with how solana works and I notice that the way nonces are used is very similar to how memos would be used (similar not in application - nonces are very different in purpose than memos - but similar in how they fit into the mechanism of adding an extra instruction to transactions). I am still trying to figure out lots of the details of how transactions are composed and submitted, but I think that very likely memo support would work very similarly to nonce support.

However, nonces appear to be a deeply embedded concept - for example, the sdk knows about them.

Do you envision memo support being that deep? Should the sdk know about them (i.e. adding a "new_with_memo" function very similar to the "new_with_nonce" function of the sdk)? Or should these be inserted into the transaction purely using functions created within the cli source code?

t-nelson commented 3 years ago

In your opinion, does the memo program deserve the same level of support as nonce?

I ask because I am still familiarizing myself with how solana works and I notice that the way nonces are used is very similar to how memos would be used (similar not in application - nonces are very different in purpose than memos - but similar in how they fit into the mechanism of adding an extra instruction to transactions). I am still trying to figure out lots of the details of how transactions are composed and submitted, but I think that very likely memo support would work very similarly to nonce support.

However, nonces appear to be a deeply embedded concept - for example, the sdk knows about them.

Do you envision memo support being that deep? Should the sdk know about them (i.e. adding a "new_with_memo" function very similar to the "new_with_nonce" function of the sdk)? Or should these be inserted into the transaction purely using functions created within the cli source code?

Memo doesn't need to be quite as deep as Durable Nonce. Durable Nonce has a few quirks that justify its specialization. For one, it is implemented as part of the system program and requires special handling in the runtime. Additionally, for a transaction to be flagged Durable Nonce, it must issue a SystemInstruction::AdvanceNonceAccount instruction as the first instruction in a transaction. This is where the Messge::new_with_nonce() helper comes in handy as it ensures that instruction is positioned correctly. Since Memo is part of the SPL, we also want to be careful not to drag that dependency all over the place. So it should be isolated to the cli/ directory in the monorepo.

How I envision this working is a new --memo MEMO_STRING (--with-memo MEMO_STRING ?) optional arg is added to all transaction bearing subcommands and if passed, a Memo instruction is appended to the normal instruction list for that transaction type. Many apologies for the plumbing hell you're about to endure. I have a major refactor planned for CLI that I haven't found time to start yet. The Memo instruction should only need to be added in one place, however due to the current architecture, each subcommand builds, signs and broadcasts its own transaction, so it will need to be added to each :face_with_head_bandage:

CriesofCarrots commented 3 years ago

Ditto what @t-nelson said. We don't want an sdk dependency on the memo program.

Or should these be inserted into the transaction purely using functions created within the cli source code?

Yes exactly, this ^

t-nelson commented 3 years ago

Or should these be inserted into the transaction purely using functions created within the cli source code?

Yes exactly, this ^

One exception might be a with_memo_arg() helper in clap-utils. I can see that being useful for maintaining consistency with the CLI tools in SPL. This won't require an spl-memo dependency though

bji commented 3 years ago

Thank you for your prompt and very helpful answers. And no worries about "plumbing hell", my only lament is that not understanding the overarching design goals (yet!) I don't feel very comfortable making structural changes that would improve this. But in terms of just using the existing patterns, I can totally do that.

bji commented 3 years ago

Sorry if this question is a bit dense but ... what would be the point of including a memo with a command line transaction? Is it just to get the memo instructions into the transaction so that they can be looked up later when examining the transaction and thus can be used as a sort of 'tag' on the transaction?

For example would I do: "solana transfer --with-memo="Payment for order 1234567" ..." and then expect later to be able to look at that transfer transaction and be able to then extract the memo data from the instruction stream and associate "Payment for order 1234567" with the transaction?

If so ... does there need to be code in the cli to do that automatically, for example in the 'transaction-history' command?

And if that is so ... when about for example 'solana vote-account'? If the vote-account were created with a transaction that included a memo, would we expect to see that memo emitted in the output of 'solana vote-account'?

bji commented 3 years ago

I wonder also, should 'solana block' decode memo data into utf-8 strings as it prints out the decoded instruction data?

mvines commented 3 years ago

I wonder also, should 'solana block' decode memo data into utf-8 strings as it prints out the decoded instruction data?

yeah! Here's a block with a memo in it: solana block 63529089

It sadly just prints this:

  Instruction 0
    Program:   Memo1UhkJRfHyvLMcVucJwxXeuD728EqVDDwQDxFMNo (4)
    Data: [49, 45, 108, 56, 51, 104, 45]
CriesofCarrots commented 3 years ago

Is it just to get the memo instructions into the transaction so that they can be looked up later when examining the transaction and thus can be used as a sort of 'tag' on the transaction?

Yes, exactly

If so ... does there need to be code in the cli to do that automatically, for example in the 'transaction-history' command?

solana confirm -v and solana block are the main locations in the cli where I would see memo exposed. We have a lot of the plumbing in place for this. Let me take a look to see which bits are missing, and I'll get back to you.

We can't really expose this in account-state queries like solana vote-account without a lot of churn to find out what transaction first created the account. Doesn't seem worth it.

I wonder also, should 'solana block' decode memo data into utf-8 strings as it prints out the decoded instruction data?

Yes, I think that would be nice.

mvines commented 3 years ago

solana confirm already knows how to handle the UTF-8 memo correctly, interesting that solana block doesn't -- this probably implies there's some duplicated code in the cli somewhere that can be cleaned up ๐Ÿงน

$ solana confirm 5EJKHPCtm1nhUZXFXti8N1kR5oKzRFa5F49v3rjh1BtmHXrtY1cewmmyanWGZSntgDDBRrsMbf64RvpEUmKmrhdL -v
RPC URL: http://api.mainnet-beta.solana.com
Default Signer Path: /Users/mvines/.config/solana/id.json
Commitment: confirmed

Transaction executed in slot 63529089:
  Block Time: 2021-02-01T14:18:46-08:00
  Recent Blockhash: 5Q93iCkZZNyhcoXutMGxE1QxeD5EMC148wU3AZbQRzq4
  Signature 0: 5EJKHPCtm1nhUZXFXti8N1kR5oKzRFa5F49v3rjh1BtmHXrtY1cewmmyanWGZSntgDDBRrsMbf64RvpEUmKmrhdL
  Signature 1: 5VFSm54NuB71ZRn4rpLPfBHt2KcqQJsU8veXhBUUq5fLB9J9fEYh9kyooWXR2C48Ng2zbikHTWZWkRUWtYA3vJ2T
  Account 0: srw- tyeraJRJFcHECfTmEGTaCzjcYebfLWETBRniRNuJN12 (fee payer)
  Account 1: sr-- Akpvka7tewujFjceSv9SifvxpY9FqRGqdkD7PiuDZVDV
  Account 2: -r-x MemoSq4gqABAXKb96qnH8TysNcWxMyWCqXgDLGmfcHr
  Instruction 0
    Program:   MemoSq4gqABAXKb96qnH8TysNcWxMyWCqXgDLGmfcHr (2)
    Account 0: Akpvka7tewujFjceSv9SifvxpY9FqRGqdkD7PiuDZVDV (1)
    Account 1: tyeraJRJFcHECfTmEGTaCzjcYebfLWETBRniRNuJN12 (0)
    Data: [240, 159, 166, 150]
  Status: Ok
    Fee: โ—Ž0.00001
    Account 0 balance: โ—Ž98.51226956 -> โ—Ž98.51225956
    Account 1 balance: โ—Ž0
    Account 2 balance: โ—Ž0.52149888
  Log Messages:
    Program MemoSq4gqABAXKb96qnH8TysNcWxMyWCqXgDLGmfcHr invoke [1]
    Program log: Signed by Akpvka7tewujFjceSv9SifvxpY9FqRGqdkD7PiuDZVDV
    Program log: Signed by tyeraJRJFcHECfTmEGTaCzjcYebfLWETBRniRNuJN12
    Program log: Memo (len 4): "๐Ÿฆ–"
    Program MemoSq4gqABAXKb96qnH8TysNcWxMyWCqXgDLGmfcHr consumed 46611 of 200000 compute units
    Program MemoSq4gqABAXKb96qnH8TysNcWxMyWCqXgDLGmfcHr success

Finalized
mvines commented 3 years ago

oops, correction. that utf shows up in the log but not in the instruction data itself

bji commented 3 years ago

OK then sorry to be further dense but ...why does the memo program have any inputs other than data then? If it's just meant to be something that gets injected into the instruction stream as tagging data, why does the program also need to have signers? Is that just so that the memo program is useful as a stand-alone program that at a minimum at least can record a text "message" that can be proven to have been vetted by the signers by virtue of being signed? But for the purposes of embedding a message in another transaction, the extra signers is not necessary ... right? I'm just thinking out loud and want to make sure I understand this.

CriesofCarrots commented 3 years ago

OK then sorry to be further dense but ...why does the memo program have any inputs other than data then? If it's just meant to be something that gets injected into the instruction stream as tagging data, why does the program also need to have signers? Is that just so that the memo program is useful as a stand-alone program that at a minimum at least can record a text "message" that can be proven to have been vetted by the signers by virtue of being signed? But for the purposes of embedding a message in another transaction, the extra signers is not necessary ... right? I'm just thinking out loud and want to make sure I understand this.

Yes, you've got that all essentially right. The signing feature in the memo was also targeting a specific use case where a client is tracking transaction logs, and wants to be able to verify the memo and its provenance.

For the this cli application, I don't think that we need to pass any signer addresses to the memo instruction.

bji commented 3 years ago

Thank you. I am so new to all of this, a little guidance really helps keep me from going down a lot of thought process dead ends!

bji commented 3 years ago

Does this look right?

https://explorer.solana.com/tx/2AFpLAbbWQDuJFp3UN5AcfHKqdPrjAPNroCKaQXpmgVoVh3kPXTLjJXCYMiYhhjPRCobshEbSSRovAsniDWSXd5w?cluster=devnet

mvines commented 3 years ago

๐Ÿ“ˆ yep!

fabiooshiro commented 2 years ago

Can I send the memo using web3.js transaction?

CriesofCarrots commented 2 years ago

@fabiooshiro , yes. There will be a npm package when this PR is complete: https://github.com/solana-labs/solana-program-library/pull/2583 In the meantime, you can use the code from memo/ts/src/index.ts directly