Extend transactions with memo sections #2358
Conversation
sug0
force-pushed
the
tiago/tx-memo-field
branch
from
b2a3789
to
afcc858
Compare
|
should we also provide a way to set this from cli? |
|
@tzemanovic absolutely, will add a cli arg for this! |
sug0
force-pushed
the
tiago/tx-memo-field
branch
from
d6f104d
to
e8d6268
Compare
There was a problem hiding this comment.
Thanks for the changes. I'm just worried about a few things: keeping the Tx size down so it can fit on the hardware wallet RAM, keeping the size of hardware wallet Tx parsers down so that they can fit in memory, assumptions about the ordering of Memo sections, and the necessary signers for a memo section to be accepted.
core/src/proto/types.rs
Outdated
| Deserialize, | ||
| )] | ||
| pub struct Memo { | ||
| inner: Vec<u8>, |
There was a problem hiding this comment.
It seems that only 3kB of RAM is available to Ledger Nano S apps. To reduce transaction size, I wonder if it's worth allowing the inner field to either be the full content or its hash by using the Commitment enumeration (like the Code section does)?
There was a problem hiding this comment.
currently, we're always submitting the full data to the hardware wallet. submitting only the commitment can be done in a separate (future) pr
sug0
force-pushed
the
tiago/tx-memo-field
branch
from
e8d6268
to
bf56adf
Compare
sug0
force-pushed
the
tiago/tx-memo-field
branch
from
bf56adf
to
d8657f0
Compare
sug0
force-pushed
the
tiago/tx-memo-field
branch
from
d8657f0
to
6bfb9a5
Compare
sug0
force-pushed
the
tiago/tx-memo-field
branch
from
6bfb9a5
to
204803c
Compare
* origin/tiago/tx-memo-field: Changelog for #2358 Regen pre-genesis tx signatures Rebuild test wasms Add tx memo CLI arg Attach memo to tx if it is present in sdk args Add memo to the sdk's tx args Extend transactions with memo sections
Describe your changes
Closes #2354
Indicate on which release or other PRs this topic is based on
v0.29.0Checklist before merging to
draft