owned this note   changed 2 years ago  
Linked with GitHub

SIGN_MODE_TEXTUAL Bi-weekly Meeting Notes

These meeting notes should be accompanied with the SIGN_MODE_TEXTUAL spec living doc: https://hackmd.io/fsZAO-TfT0CKmLDtfMcKeA?edit. This has been merged into ADR-050.

Implementation tracking issue: https://github.com/cosmos/cosmos-sdk/issues/11970

Meeting 05.06.2023

  • [] Ledger App
  • Audit finished (and we have a report)

Meeting 22.05.2023

  • [] Ledger App
  • [] Audit finishing (writing the report)

Meeting 24.04.2023

  • Ledger App (no updates)
  • Audit
    • Zellic

Estimated Project Timeline
Pre-Audit Preparation: Up until May 2, 2023
Scope Call: May 2, 2023
Primary Review Period: May 2, 2023 to May 23, 2023
Closing Call: May 24, 2023
Post-Audit Remediation: May 24, 2023 to June 14, 2023

Other:

Notes:

  • signmode textual in CosmJS in Q3 probably
  • Jim will be backporting to Agoric

Meeting 10.04.2023

  • Ledger App
  • Audit (no news yet)
  • PRs
    • Amaury's PR #15550
    • Orijtech fuzz testing
      Image Not Showing Possible Reasons
      • The image file may be corrupted
      • The server hosting the image is unavailable
      • The image path is incorrect
      • The image format is not supported
      Learn More →
    • Facu will have a PR ready for this week with small changes

Questions:

  • It must be able to parse all valid screens, however the behavior of Parse on invalid screens is not specified, and does not necessarily error. ??

Notes:

  • Parse is currently used only for testing

Meeting 27.03.2023

  • Audit
    • Zellic
  • Ledger app

Meeting 20.03.2023

Notes:

  • ledger app:
    • OK to submit ledger app to the ledger repo
  • Internal audit
    • Facu and Amaury to share this task

Meeting 13.03.2023

Notes:

  • Ledger
    • Carlos implemented array to
    • Facu and Amaury to test it
type SignDoc struct {
  Screens []Screen
}
  • Keep struct
  • Current version == 0 (update in the PR)

Meeting 06.03.2023

Notes:

  • Ledger app:
    • Facu to do last MsgMultiSend test
    • then we can submit the PR
    • Still some (maintenance) code to be updated for the ledger repo
  • Spec

We decided to do:

type SignDocTextual = { screens: []Screen } // integer-encoded key

Meeting 20.02.2023

Notes:

  • Ledger: last pieces before sending the app to Ledger:
    • MsgDelegate fixed
      • carlos to send the installer script, amaury to test it
    • msgMultiSend: crashes because Json parsing number of tokens got reduced, to handle Textual
      • Carlos: looking into increase number of tokens
    • tracking the >>> issue with Facu
  • Next Meeting: March 6th
  • Spec:
    • Spec updates:
      • Simon: multi-chain client needs to know version of the Textual spec
      • Simon: spec version counter
      • Simon: no need for endpoint to expose version for now, keep it simple.

Meeting 13.02.2023

Notes

  • Amaury and Facu to finish manual testing, ideally this week
  • Right after: send PR to Ledger
  • Internal audit:
    • Jim: confident in core value renderer code, less confident in 1/ plumbing code with SDK signing 2/ number of screens
  • SPEC updates
    • tweaks to formatting allowed, should be expected, needs to documented (state-machine breaking)
    • changes that impact ledger cosmos app should be minimized as much as possible

Meeting 06.02.2023

  • Ledger App
  • Spec
  • PRs
    • any msg header
    • title/content follow-up

Notes:

  • Ledger
    • Send PR now VS wait for audit?
      • Fernando: send asap
      • Amaury/Marko: send asap
      • Estimated timeline: in 2 weeks

Meeting 30.01.2023

Notes:

  • Ledger
    • v2.35.2 Looks good
    • Ledger review process can be uncertain, sometimes our tests pass theirs don't
    • might be a good idea to do audit of ledger app
    • Process:
      • create PR on ledger repo
      • they run fuzzers
      • they have more user-focused testing
    • Textual not available, but if we provide a document/tutorial/spec, it should be fine
    • Ask Ainhoa for feedback about audit + length review length

Meeting 23.01.2023

Agenda:

Notes

Meeting 16.01.2023

Agenda:

Notes

  • SPEC Remove the address as it's redundant (SDK code, golang) in the envelope
    • mark as expert
  • Truncate:
    • Done on ledger
    • Concern 1: different fields, same truncated text
    • Concern 2: important semantic info truncated, e.g. from_address_to_exclude

Going with this format:


|>>> --- [1/2]
|From address: cosmos1sdafdsa
|dafdsfdasfdsa
---
|>>> --- [2/2]
|sdafdsafdsfdsa
|dafdsfdasfdsa

Define length on Nano S and Nano S plus to start truncating.

Meeting 09.01.2023

Agenda:

Notes:

  • No updates on the ledger app
  • Amaury: this week make it work with Ledger app

Meeting 19.12.2022

Notes:

Meeting 12.12.2022

Notes

  • Ledger
    • Carlos: version working, but not on nano s becuase of memory constraints
{
 "text": ...,
 "expert": bool,
 "indent": int
}[]

"Memo: foo" gives

------------
|  Memo:
|  Foo
---------

Idea:

{
 "title": string, // max 11 chars
 "content": string
 "expert": bool,
 "indent": int
}[]

  • UTF-8 in strings?

    • Zondax: ideally not

  • Make titles shorter (e.g. Msg instead of Message)

  • Simon: maybe remove AMION_JSON support for nano S, only do Textual

    • makes no sense for legacy nano s to define Textual spec

  • Simon: doubt that CBOR addition leads to out of memory. In the long-term would prefer not to use JSON

  • Jim: see why AMINO_JSON is shorter

  • Solutions:

    1. Let the server do some more handling (unicode->ASCII, title/content, shorter screens)
    2. Continue w/ current spec
      2a. Only support the minimum for nano s
      2b. 2 differents apps for nano s.

  • Fernando: 2a is preferred, if nano s support 80%-90% of txs

  • Amaury: 2a

  • Simon: 2a is best, we can do 1 if we don't harm future spec because of legacy nano s txs.

    • e.g. title/content spec makes it better
    • not for unicode

  • Jim: is it code memory pressure or data memory pressure? Or same resource pool?

    • code memory and data memory separate
    • We can add more code to help reduce the runtime memory

Action items

  • try a big tx that's signable with AMINO_JSON
  • double check: large tx passes on nano X and s plus
  • Create issue for title/content @amaurym

Meeting 05.12.2022

Vote:
> Voter: cosmos1

Not okay

Vote: [Vote object
> Voter: cosmos1
]

current one

Vote: /cosmos.gov.v1.Vote
> Voter: cosmos1

too long

Notes

  • Ledger
    • Fernando: stack overflow on nano s
    • related to JSON library
    • possible solution: no AMINO_JSON on nano S
    • Jim: fine to have both apps?
      • possible yes
    • Fernando: priority to put 2 sign modes in one app
    • Jim: would we have stack overflow without CBOR?
    • Fernando: not sure
    • Jim: option: idea to reuse JSON instead of CBOR (easy fix)
    • Fernando: ideally have a answer whether both sign modes in one app (nano s) is possible
    • Carlos: no issues on guidelines and tst cases

Meeting 28.11.2022

Agenda:

  • Ledger updates
    • Do the guidelines and test vectors make sense?
    • @ as terminator for whitespaces
    • \u for unicode (except special chars)
    • pagination
  • PRs (walk through the tracking issue)

Notes:

https://github.com/cosmos/ledger-cosmos/blob/main/tests_zemu/snapshots/s-sign_basic/00001.png

------------
|  <title>
|  <content>
---------

------------------------
|  <no title>
|  > End of transactions
-------------------------

Action item:

  • amaurym create edge case no : separator and long screen

Meeting 21.11.2022

Agenda:

  • Ledger updates
  • PRs (walk through the tracking issue)

Notes:

  • Fernando:
    • good progress with parser
    • need more test vectors
    • Good to star twith 2/3 test casees
    • Before sending to Ledger:
      • much more test cases, with edge cases
      • maybe with a tool to generate those test cases

Action Items

  • amaury: create a JSON file with 2-3 test cases
    • try to cover corner cases, e.g. with a lot of fields
    • differents txs with different fields

JSON file:

{    
    "tx": "<binary_repr>" // CBOR-encoded
    "output": [
        "<screen1>",
        "<screenN>", 
    ],
    "expert": true
}

Meeting 14.11.2022

[{denom:"ATOM",amount:1},{denom:"uatom",amount:1000000}]

Current: 1 ATOM, 1 ATOM

Conclusion: no special handling of coins, all test cases (and parse) use base denom

  • Notes

  • CosmJS

    • currently not active, not in foreseeable future
    • framing: want Library developer (language is secondary)
    • confio: more a mgmt problem than a talent problem (for newcomers)

Meeting 07.11.2022

Notes

  • Ledger:

    • Support both AMINO_JSON and TEXTUAL (ideal solution)
    • Fernando: Checkout about Nano S memory
    • PR (draft) on https://github.com/cosmos/ledger-cosmos
    • No need for determinism check on the ledger decoding side
    • strings vs integers on struct fields for CBOR:
      • Fernando: both are fine
      • Jim: keep as integers
    • Timeline:
      • Ainhoa: should be fine by Christmas for a testable version of the app
      • Not for release
      • Jim: would love to have this asap for Agoric
    • Monday 5:30pm CEST

  • Coins

  • map: denom => base_denom

  • MsgSend {
    Amount: {denom: "atom", amount: 1} // user does not have funds
    }

  • Conclusion: add comments in Parsing that we parse back to base denom (and not original denom) (problem for future us)

  • Joe: probably too late, but https://cbor.me/ has been helpful in the past

    • to be used to decode cbor, probably useful for testing

  • Proto annotations

https://github.com/cosmos/cosmos-sdk/pull/13466/files

string foo = 1 [cosmos_proto.scalar="cosmos.Dec"];

cosmos/scalars.proto

declare_scalar = {

}

alternative:

string foo = 1 [cosmos.textual.v1.value_renderer="Dec",
cosmos.amino.v1.encoding="Dec"]];

conclusion: let's go with scalars

Meeting 24.10.2022

Notes:

  • Ledger app
    • Juan: maybe early November
    • Juan: needs test vectors
      • inputted data: CBOR-encoded
      • output: []string (with or without expert)
    • Juan: get developer to join biweekly syncs
    • Jim: Still need to specify CBOR encoding
    • Juan: one ledger app to support both
    • derivation path:
      • Simon: it's on hold, out of scope of Textual
    • Trezor: see if they are interested

Action items;

  • create issues for integration with existing signing flow @amaurym
  • call for in 2 weeks

Meeting 17.10.2022

Action Item:

  • next time(s): Zondax team member to join call

Meeting 03.10.2022

  • Spec PR with new strcutured screens (Jim)
    • ping Simon Amaury Juan to review, secondary Joe, Emmanuel
  • Duration PR
  • Messages PR (Jim)
  • Ask Juan about Annex 2:
    • does Juan want exact spec, or general guidelines?

Meeting 14.09.2022

Juan, Jim, Amaury

What Ledger device receives

Before:

\n-delimited string
each screen: ASCII 32-127, e.g. *, >

After:

Item {
    "expert": true,
    "indentation": 5, // >>>>>
    "string": "..." // can contain any UTF-8, can contain >, *, \n
}

SignDoc = []Item

What Ledger receives:
enc(SignDoc) // where enc is a determinitic encoding fn

What should enc be:

Decision: CBOR (make sure about determinism)

UTF8

  • in other projects:
    • replace with . for everything outside 32-127
  • other ideas:
  • other idea: keep ASCII in spec, hardware devices that can show UTF-8 can also decode on-device

\n-delimiter string vs CBOR(): Juan prefers CBOR

If we go CBOR: Juan prefers escpaed utf8 in the serialized Item.string

Decision:

  • go with CBOR
  • in the Item.string field, can be full utf-8
  • During serialiazation, Item.string quoted to printable ASCII 32-127

To check: is CBOR's string an ASCII string or UTF-8 string?

Other questions

  • Maximum expected size of txs

    • cosmwasm contracts
      • maybe sign hash (to be discussed next time)
      • maybe streaming
    • agoric: use custom msg renderers to shorten the Item

  • What to do with long values (e.g. long memo field)?

    • not monospace font
    • sensible limit: 255 ASCII chars (after encoding)

Meeting 12.09.2022

Agenda

Spec:

  • Scope of the SPEC
    • What sign_mode_textual produces
    • Which parts are left to the signing device

Implementation PRs:

Notes

  • SIGN_MODE_TEXTUAL spec:
Item {
    "expert": true,
    "indentation": 5,
    "string": "..." // can contain any UTF-8, can contain >, *, \n
}

SignDoc = []Item

Encoding of SignDoc: (needs to be deterministic)
- micro-format
- CBOR
- NEAR binary format (https://borsh.io/)

Action Items

  • Ask Juan about:
    • encoding of SignDoc
    • UTF-8 quotation

Meeting 05.09.2022

Agenda:

Spec:

Implementation PRs:

Action items:

  • Ask Juan about converting UTF8 chars to display code (string -> string literal, use qutoation marks, similar to JSON) inside the Ledger App.

Meeting 22.08.2022

Agenda:

Notes:

  • Security:
    • Today: if we merge above PRs, then nothing to do
    • Before shipping:
      • internal (SDK team) audit SPEC+impl
      • external (cosmos ecosystem) audit SPEC+impl
    • bijectivity: part of security
  • Bijectivity:
    • Simon: do we really need it?
    • Jim:
        1. sigs are not malleable
        1. sig over readable tx data
  • Hex or base64 for pubkeys?

Action Items:

  • (from last week) Update SPEC with * and > positioning
  • change bijectivity to SHOULD. Bijectivity between ``"Semantic Tx" -> []string`
  • create issue on SDK to ask about hex vs base64.
  • Jim: Custom Msg reneder PR: push to finish line

Meeting 08.08.2022

Agenda:

MsgFooBar {
  // link to spec custom render = 
  custom_renderer = "algo_id"
  
  
}

Notes:

  • Clear up SPEC on asterisks
    • add recursive asterisks spec (only 1 asterisk)
    • one asterisk for multiple nested expert fields
    • * or >: * at beginning
      • *> Amount: 20 atom
    • a regular formatted string should not start with '*' or '>'
  • Add that custom_renderer proto option is optional

Meeting 25.07.2022

Agenda:

Notes:

  • Issue: https://github.com/cosmos/cosmos-sdk/issues/11970
    • assigned Jim and Joe to some subtasks
  • For custom renderers:
    • Jim: prefer the custom function
    • Amaury: okay for this if we can make this function bijective, or else fall back to hidden fields.
  • Internationalization:
    • Amaury: update ADR
  • amaury: ask marko to add JimLarson to sdk-write

Meeting 11.07.2022

Agenda:

  • Expert mode
  • Implementation
message MsgMyExample {
    string user_string = 1;
    string address = 2 [exprert=true];
    // other fields [exprert=true];
}

message MsgMyExample3 {
    [option expert_all_but="user_string"]
    string user_string = 1;
    string address = 2;
    // other fields;
}

message MsgMyExample2 {
    [option readable_string="...{{address}}..."]
    string address = 2;
    // other fields;
}

(template engine not expressive enough for agoric's use cases)

Action Items:

  • Simon to think about user-defined function inside proto file to do Msg -> string encoding

Meeting 16.05.2022

Agenda:

Meeting 21.03.2022

import { gluecode } from '@cosmjs/core|gluecode'

import authz frm '@cosmjs/authz'
import ecocredits frm '@regenjs/ecocredis'


export const client = gluecode([
  authz,
  ecocredits,
])

// publish this client

Action items

  • amaury: add negative numbers, ask around if there's a use case

Meeting 07.03.2022

Notes:

Meeting 21.02.2022

  • Tips check-in
  • Max length on each screen?
  • Start implementation

Notes

  • tips:

    • Juan talked with Billy, moved to cosmos org, tip support added
    • on cosmjs: no timeline. Bad quality external contributors, can ask Emeris team to add, but needs to be good quality (advanced TS engineers).
      • if there's really urgent need, Zondax can help.

  • Max length

    • nano s max tx size == 8kb
    • Gov submit proposal metadata, change to string?
    • additional restrictions: acsii range,
    • no risk, ledger returns an error if tx too big. no need to put artifical limit here

  • Implementation

    • Metadata denom: don't change capitilization, keep what's in state
    • what happens to TEXTUAL on state change? Will make offline signing harder if state changes often, but for metadat it should be okay,

Action Items

  • @amaurym Gov submit proposal metadata, change to string?

Meeting 07.02.2022

Agenda:

  • transaction tips check-in

Recall from 12.10.2021:

{
  "account_number": {number},
  "chain_id": {string},
  "fee": {
    "amount": [{"amount": {number}, "denom": {string}}, ...],
    "gas": {number},
+   "granter": {string}, // optional, omitempty, chain-specific bech32 address
+   "payer": {string}, // optional, omitempty, chain-specific bech32 address
  },
  "memo": {string},
  "msgs": [{arbitrary}],
  "sequence": {number},
+  "tip": { // optional, whole struct is omitempty
+    "amount": [{"amount": {number}, "denom": {string}}, ...], // array of coins
+    "tipper": "cosmos1...",
+  }
}

From Zondax:

  • moving the Ledger app code to the cosmos org

  • end feb seems like a reasonable timeline for code-readiness. Then unknown time for Ledger team approval.

  • textual check-in

No reply from Ledger team on Discord. Amaury will ping Sam/Billy if nothing moves.

Meeting 24.01.2022

Agenda:

  • check-in Ledger
  • Nested Msgs with >
  • Links (e.g. propsal on MsgVote)

Action items;

  • amaury to send a msg to billy/sam for comms with Ledger
  • start impl (Simon & Amaury) once we have a ledger ACK

Meeting 10.01.2022

Agenda:

  • If we get ACK from ledger, start implementation?
  • Nested Msgs with >
  • Links (e.g. propsal on MsgVote)
  • SPEC

Notes:

  • Ledger still playing hard to get, Amaury and Juan to continue harassing
  • Once Ledger says OK, Simon & Amaury can start a couple of hours per week on paired impl.
  • Simon: important to have multi-langugage impl, to iterate with SPEC (and not have impl-as-SPEC)
  • Amaury: share test cases between go & js
  • OK to go with > for nested messages, put it in spec (would love to have Juan's input on)
  • Amaury to continue write Value Renderers in a SPEC-y language

Meeting 13.11.2021

Agenda:

Notes:

Meeting 29.11.2021

Agenda:

  • BC Vault update
  • SPEC

Action:

  • Create draft PR, so that some decisions can be discussed asyncly

Meeting 15.11.2021

Agenda:

  • Ledger team schedule call (or in-person event)
  • Continue on Spec

Notes:

  • make Ledger understand that we need to get on a call w/ them

Action item:

  • @corlock reach out to Billy/Sam to contact Ledger
  • @amaurym ping denis to contact BCVault

Meeting 08.11.2021

Agenda:

  • Meeting frequency
    • Ok to swithc to biweekly starting from 15th Nov
  • Go through the initial design doc
  • In value renderers for numbers: consider using ' for 1000s separator. (depends on language, find compromise)
  • Simon: use consistent language, prefer full words (e.g. transaction over tx, message over Msg)
  • What if user put \n in memo?
    • Juan: propose to only allow ASCII 32-127

  • Each string is less than N (TBD) characters, to avoid string streaming on the screen.

    • More relevant is pixel size. "m" is wider then "i"
  • Think about the realisitic max number of lines and max number of chars/line.

Meeting 04.10.2021

Agenda:

  • Make sure everyone is aware we're going with SIGN_MODE_AMINO_AUX for SDK v0.45 (~Nov).

  • Existing LEGACY_AMINO_JSON:

type StdSignDoc struct {
	AccountNumber uint64
	Sequence      uint64
	TimeoutHeight uint64
	ChainID       string
	Memo          string
	Fee           json.RawMessage
	Msgs          []json.RawMessage
}
  • New AMINO_AUX
type StdSignDocAux struct {
	AccountNumber uint64
	Sequence      uint64
	TimeoutHeight uint64
	ChainID       string
	Memo          string
	Msgs          []json.RawMessage
    Tip           sdk.Coins // [{"amount":"1000", "denom":"atom"}]
}
  • Temperature check on whether the updated SIGN_MODE_TEXTUAL is a good starting point.

Notes:

Action Items:

Select a repo