From 58fb3aecba14b1c2e3761d80bd5eb2d256fbc5b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Carneiro?= Date: Thu, 28 Mar 2024 00:18:19 -0300 Subject: [PATCH 1/8] feat: token admin methods --- .../ledger-app/0001-token-admin-methods.md | 205 ++++++++++++++++++ 1 file changed, 205 insertions(+) create mode 100644 projects/ledger-app/0001-token-admin-methods.md diff --git a/projects/ledger-app/0001-token-admin-methods.md b/projects/ledger-app/0001-token-admin-methods.md new file mode 100644 index 00000000..8bc41543 --- /dev/null +++ b/projects/ledger-app/0001-token-admin-methods.md @@ -0,0 +1,205 @@ +# Summary +[summary]: #summary + +Support for admin operations for custom tokens on the Ledger app. +This includes: +- Creating tokens and NFTs. +- Minting new tokens and NFTs. +- Melting new tokens and NFTs. +- Delegating authorities. +- Destroy authorities + +# Motivation +[motivation]: #motivation + +Today Hathor's Ledger app only allow sending transactions with custom tokens, so users and use-cases can hold and send custom tokens, but to create and manage supply it is required to use the software wallet. +This design is to allow uses who use Ledger, a hardware cold wallet, to fully manage custom tokens without using any software wallet. + +# Guide-level explanation +[guide-level-explanation]: #guide-level-explanation + +> [!WARNING] Request for comment +> This whole paragraph below can change if we really want to, we just need to add an `isNFT` boolean in the token data for the user to confirm, so in all transactions the Ledger will be informed that the token is an NFT with the user express consent during token signing +> This makes a breaking change in the wallet dekstop since we will change an existing command. + +Before entering the operations, we should address the differences between custom tokens and NFTs. +In Hathor, NFTs are custom tokens, where the creation transaction (which is the uid of the NFT) has the NFT data associated which is usually an IPFS link to the NFT document (image, pdf, video, etc.). +After the creation the operations and transactions involving NFT are exactly the same regarding the protocol, but the balance is shown differently to the user, where the custom token unit is worth 0,01 the NFT is always integer, so it is 1. Both of these are the same on the transaction, it is just different on the user prompt. +Since the Ledger cannot check if a token is an NFT or not, we should disregard any changes and treat both as custom tokens (which is valid if we follow the protocol). + +## Transaction parsing + +To allow admin operations we will introduce new concepts into the app that were being ignored or not treated. + +--- + +The `token_data` of an output is a byte with the information: +- Weather the token is a custom token or HTR +- Which custom token it is (index on the token array) +- If the output is an authority output (which authority will be encoded on the `value`) +Currently on the app we ignore the last piece of information since we do not allow authority outputs. +The way we do it is by throwing an error of the `token_data & TOKEN_DATA_AUTHORITY_MASK` is not 0. +To allow authority outputs we just need to change this and create a special screen for authority outputs when the user is confirming them. +Usually the output confirmation screen involves 4 steps: +- Address step (with pagination) +- Amount step (Showing as `ABC 0.01` or `HTR 0.01`, with pagination for big values) +- Approve step +- Reject step + +When confirming an authority output we should show: +- Address step (with pagination) +- Type step (`mint authority` or `melt authority`) + - Other authorities will throw. + - We will not allow joined authority, for instance `mint+melt authority` (i.e. `value = 3`) +- Approve step +- Reject step + +Obs: With authority outputs we already have enough to enable mint, melt and delegate operations. + +--- + +Currently we only allow P2PKH scripts (and addresses) but NFTs have a data output. +Data outputs are allowed on the protocol but we will only allow on NFT creation scripts because any other is just an HTR burn. +Data outputs are allowed to use custom tokens on the protocol but we will only allow HTR (token index 0) since NFT creation requires HTR. + +The user will be required to confirm the data on the script, which means that only ascii characters will be allowed. + +--- + +When creating a custom token or NFT the transaction information also includes the name and symbol of the token, these should only exist when a transaction has the version byte 2. + +--- + +> [!NOTE] Version field has been split into `signalBits` and `version` +> The version field of the transaction was 2 bytes (first byte was reserved for later use) and since Ledger app version 1.1.0 the version field has become a 1 byte field and the first byte became the `signalBits` + +Version byte needs to be refactored to a 1 byte field and support for `signalBits` will not be added in this design so we will only allow `0x00` as `signalBits` for now. +## Admin operations + +### Mint, Melt and Delegate + +Sending a mint, melt and delegate transactions should use the same protocol as sending normal transactions. +Mint and Melt involve destroying/creating custom token outputs and since the Ledger app does not have access to the input data it can only see the outputs, which means that a mint operation is a transaction with custom tokens outputs (possibly HTR outputs for change) and authority outputs, same for melt operations. + +Delegate are simpler since they actually are just sending an authority output. + +### Create token + +Token creation transactions are special for a number of reasons: +1. Transaction version byte is `0x03` +2. Extra fields at the end of the transaction, namely `token_info_version`, `name` and `symbol` +3. There will be an output with the created token, the token uid will not be on the tokens array since it does not exist yet + 1. Token uid is the transaction id of the created transaction, which is created during mining. + +To deal with these we will create a new command to load the token data of the token being created. +We will use the existing `SEND_TOKEN_DATA` for instruction code `0x08` and use the `P1` field to indicate which data is being sent. + +| Description | P1 | P2 | CData | +| ------------------------------------------- | ---- | ---- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Send token data
for signing transactions | 0x00 | 0x00 | `version (1)` \|\|
`uid (32)` \|\|
`symbol_len (1)` \|\|
`symbol (symbol_len)` \|\|
`name_len (1)` \|\|
`name (name_len)` \|\|
`signature (32)` | +| Send token data
for creating a token | 0x01 | 0x00 | `version (1)` \|\|
`symbol_len (1)` \|\|
`symbol (symbol_len)` \|\|
`name_len (1)` \|\|
`name (name_len)` | +| Send token data
for creating an NFT | 0x01 | 0x01 | `version (1)` \|\|
`symbol_len (1)` \|\|
`symbol (symbol_len)` \|\|
`name_len (1)` \|\|
`name (name_len)` \|\|
`data_len (1)` \|\|
`data (data_len)` | +The new commands will require a new flow for user confirmation +1. Screen showing the user this is a "Create token" data. +2. Screen with the symbol +3. Screen with the name +4. If creating an NFT a screen with the data (ascii encoded), paginated +5. Approve screen +6. Reject screen + +# Reference-level explanation +[reference-level-explanation]: #reference-level-explanation + +Authority outputs can be inferred through the `token_data` and `value` so we should not add extra fields but methods to check if an output (`tx_output_t`) is an authority and which authority it is. + +```c +bool is_authority_output(tx_output_t output) { + return (output.token_data & OUTPUT_AUTHORITY_MASK) > 0 +} + +bool is_mint_authority(tx_output_t output) { + return is_authority_output(output) && ((output.value & MINT_AUTHORITY_MASK) > 0); +} + +bool is_melt_authority(tx_output_t output) { + return is_authority_output(output) && ((output.value & MELT_AUTHORITY_MASK) > 0); +} +``` + +For data outputs we will introduce a new `data` field which will hold the actual data script. +We should fail if the data script is not ascii. + +```c +typedef struct { +    uint8_t index; +    uint64_t value; +    uint8_t token_data; +    uint8_t pubkey_hash[PUBKEY_HASH_LEN]; +    // New data field +    uint8_t data[MAX_DATA_SCRIPT_LEN] +} tx_output_t; +``` + +--- + +During the transaction processing we use the method `receive_data` which internally calls `_decode_elements` untill we reach a fully formed output (which triggers a user confirmation) or we reach the end of the buffer (which returns a request for more data) when we parse all outputs (as defined in the initial portion of the wallet) and there is no more data to parse we start the UI step to request the user for a final tx confirmation. + +To add support for our new transactions we require some changes on output confirmation. + +### Output confirmation + +The method `ui_display_tx_outputs` prepares the outputs (address base58 encoding, value formatting for UI, etc.) and shows the screens with the output information to the user (with `ux_display_tx_output_flow`). +To adjust this we need to check if the output is a P2PKH output, a data output or an authority output, each output type have different data for the user to check so it makes sense that each has its own flow (i.e. screens that display information and ask confirmation or rejection). + +- normal P2PKH outputs + - `pubkey_hash` field of the output should not be empty. + - `token_data` should indicate that this **IS NOT** an authority output. + - This is the current supported format. + - For a token creation, we can check if this is the token being created and use the loaded symbol for the amount. +- authority P2PKH outputs + - `pubkey_hash` field of the output should not be empty. + - `token_data` should indicate that this **IS** an authority output. + - Should confirm the address and authority of the output. + - The authority step should have the title "Authority" and text either "Mint \" or "Melt \" depending on the type of authority. +- Data outputs + - Should only exist if the transaction version byte is `0x03` + - `data` should be ascii safe + - `value` should always be 1 + - No need for confirmation, we just need to check that the data matches the user confirmed data. + +### Transaction parsing + +The current process to parse the transaction will parse the 2 version bytes then the length of tokens, intputs and outputs (1 byte each) after this we parse the token uids, inputs and outputs as the length bytes described. +For "create token transactions" (identified by version byte `0x03`) we have some extra fields at the end of the transaction, these fields should already be confirmed by the user so we just need to make sure the transaction information matches the user confirmed data. +# Rationale and alternatives +[rationale-and-alternatives]: #rationale-and-alternatives + +### Separate create tx command + +We could create a separate command for create token transactions, but the transaction parsing methods are very complex and duplicating this code is not desirable. + +### Do not create a new `SEND_TOKEN_DATA` command + +Instead of the new command to load the token data being created we could just confirm the data when they appear on the transaction parsing, this would create an issue for outputs containing the token being created since during parsing the symbol is unknown (since it is the last piece of information) so we would need to show the amount as "Amount created" or similar (same with authorities) to signify that the output is of the created token. + +# Unresolved questions +[unresolved-questions]: #unresolved-questions + + + +# Future possibilities +[future-possibilities]: #future-possibilities + +### Nano contract + +For signing we just require an extra signature for the caller that can be called during the usual process. +The issue is that we need to parse the nano contract data and add a confirmation step for these. +Since the arguments can be very technical the app UX will not be very user friendly. +This UX challenge should be addressed in its own design. + +### P2SH + +We currently only support P2PKH scripts and we are adding support for data scripts (under strict conditions). +The difficulty with P2SH is the lack of the `redeemScript` which is required to derive addresses, since we would require the multisig configuration to be loaded in the app and we do not have support for this yet. +Without address derivation we cannot check that any addresses are from our wallet (making change output checks fail). +Also, we do not have permission to derive in the Hathor multisig path derivation (see [here](https://github.com/HathorNetwork/hathor-wallet-lib/blob/v1.4.0/src/constants.ts#L267)) so this would also need to change. \ No newline at end of file From 2cbd341848295d730d9a92fe8900d9aebdb483ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Carneiro?= Date: Thu, 28 Mar 2024 00:22:13 -0300 Subject: [PATCH 2/8] chore: missing new line --- projects/ledger-app/0001-token-admin-methods.md | 1 + 1 file changed, 1 insertion(+) diff --git a/projects/ledger-app/0001-token-admin-methods.md b/projects/ledger-app/0001-token-admin-methods.md index 8bc41543..4a8a53b2 100644 --- a/projects/ledger-app/0001-token-admin-methods.md +++ b/projects/ledger-app/0001-token-admin-methods.md @@ -99,6 +99,7 @@ We will use the existing `SEND_TOKEN_DATA` for instruction code `0x08` and use t | Send token data
for signing transactions | 0x00 | 0x00 | `version (1)` \|\|
`uid (32)` \|\|
`symbol_len (1)` \|\|
`symbol (symbol_len)` \|\|
`name_len (1)` \|\|
`name (name_len)` \|\|
`signature (32)` | | Send token data
for creating a token | 0x01 | 0x00 | `version (1)` \|\|
`symbol_len (1)` \|\|
`symbol (symbol_len)` \|\|
`name_len (1)` \|\|
`name (name_len)` | | Send token data
for creating an NFT | 0x01 | 0x01 | `version (1)` \|\|
`symbol_len (1)` \|\|
`symbol (symbol_len)` \|\|
`name_len (1)` \|\|
`name (name_len)` \|\|
`data_len (1)` \|\|
`data (data_len)` | + The new commands will require a new flow for user confirmation 1. Screen showing the user this is a "Create token" data. 2. Screen with the symbol From 06aadd145b7ad2c0ced3a3901ed540807f305e66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Carneiro?= Date: Thu, 28 Mar 2024 10:41:01 -0300 Subject: [PATCH 3/8] chore: add destroy operation --- projects/ledger-app/0001-token-admin-methods.md | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/projects/ledger-app/0001-token-admin-methods.md b/projects/ledger-app/0001-token-admin-methods.md index 4a8a53b2..a7f7ce32 100644 --- a/projects/ledger-app/0001-token-admin-methods.md +++ b/projects/ledger-app/0001-token-admin-methods.md @@ -54,7 +54,7 @@ When confirming an authority output we should show: - Approve step - Reject step -Obs: With authority outputs we already have enough to enable mint, melt and delegate operations. +Obs: With authority outputs we already have enough to enable mint, melt, delegate and destroy operations. --- @@ -62,7 +62,7 @@ Currently we only allow P2PKH scripts (and addresses) but NFTs have a data outpu Data outputs are allowed on the protocol but we will only allow on NFT creation scripts because any other is just an HTR burn. Data outputs are allowed to use custom tokens on the protocol but we will only allow HTR (token index 0) since NFT creation requires HTR. -The user will be required to confirm the data on the script, which means that only ascii characters will be allowed. +The user will be required to confirm the data on the script, which means that only ascii characters will be allowed since Ledger does not support UTF-8. --- @@ -74,14 +74,18 @@ When creating a custom token or NFT the transaction information also includes th > The version field of the transaction was 2 bytes (first byte was reserved for later use) and since Ledger app version 1.1.0 the version field has become a 1 byte field and the first byte became the `signalBits` Version byte needs to be refactored to a 1 byte field and support for `signalBits` will not be added in this design so we will only allow `0x00` as `signalBits` for now. + ## Admin operations -### Mint, Melt and Delegate +### Mint, Melt, Delegate and Destroy -Sending a mint, melt and delegate transactions should use the same protocol as sending normal transactions. +Sending a mint, melt, delegate or destroy transactions should use the same protocol as sending normal transactions. Mint and Melt involve destroying/creating custom token outputs and since the Ledger app does not have access to the input data it can only see the outputs, which means that a mint operation is a transaction with custom tokens outputs (possibly HTR outputs for change) and authority outputs, same for melt operations. -Delegate are simpler since they actually are just sending an authority output. +Delegate is simpler since its actually just sending an authority output. + +Destroy operations usually do not even have authority outputs, so it actually is already supported. +Granted that the token array will have the token uid of the authority being destroyed so the user still has to sign the token data even with no outputs of the token. ### Create token From 53f0a0de8ca06d213d86b7daeb8b903f700f49bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Carneiro?= Date: Sat, 30 Mar 2024 12:10:46 -0300 Subject: [PATCH 4/8] chore: typo --- projects/ledger-app/0001-token-admin-methods.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/projects/ledger-app/0001-token-admin-methods.md b/projects/ledger-app/0001-token-admin-methods.md index a7f7ce32..97644620 100644 --- a/projects/ledger-app/0001-token-admin-methods.md +++ b/projects/ledger-app/0001-token-admin-methods.md @@ -7,13 +7,13 @@ This includes: - Minting new tokens and NFTs. - Melting new tokens and NFTs. - Delegating authorities. -- Destroy authorities +- Destroying authorities. # Motivation [motivation]: #motivation -Today Hathor's Ledger app only allow sending transactions with custom tokens, so users and use-cases can hold and send custom tokens, but to create and manage supply it is required to use the software wallet. -This design is to allow uses who use Ledger, a hardware cold wallet, to fully manage custom tokens without using any software wallet. +Today Hathor's Ledger app only allow sending transactions with custom tokens, so users and use-cases can hold and send custom tokens but to create and manage token supply it is required to use a software wallet. +This design is to allow users who use Ledger, a hardware cold wallet, to fully manage custom tokens without using any software wallet. # Guide-level explanation [guide-level-explanation]: #guide-level-explanation @@ -21,10 +21,11 @@ This design is to allow uses who use Ledger, a hardware cold wallet, to fully ma > [!WARNING] Request for comment > This whole paragraph below can change if we really want to, we just need to add an `isNFT` boolean in the token data for the user to confirm, so in all transactions the Ledger will be informed that the token is an NFT with the user express consent during token signing > This makes a breaking change in the wallet dekstop since we will change an existing command. +> We can avoid this command change by using the P1-P2 to create a SEND_TOKEN_DATA v2 where we have the `isNFT` and keep the v1 unchamged. Before entering the operations, we should address the differences between custom tokens and NFTs. In Hathor, NFTs are custom tokens, where the creation transaction (which is the uid of the NFT) has the NFT data associated which is usually an IPFS link to the NFT document (image, pdf, video, etc.). -After the creation the operations and transactions involving NFT are exactly the same regarding the protocol, but the balance is shown differently to the user, where the custom token unit is worth 0,01 the NFT is always integer, so it is 1. Both of these are the same on the transaction, it is just different on the user prompt. +After the creation the operations and transactions involving NFT are exactly the same regarding the protocol, but the balance is shown differently to the user, where the custom token unit is worth 0,01 the NFT is always integer, so it is 1. Both of these are the same value on the transaction it is just shown different on the UI. Since the Ledger cannot check if a token is an NFT or not, we should disregard any changes and treat both as custom tokens (which is valid if we follow the protocol). ## Transaction parsing From 234b68e860e66c6dd2cce93b2851d7844c08538e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Carneiro?= Date: Wed, 3 Apr 2024 12:05:13 -0300 Subject: [PATCH 5/8] chore: add task breakdown --- projects/ledger-app/0001-token-admin-methods.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/projects/ledger-app/0001-token-admin-methods.md b/projects/ledger-app/0001-token-admin-methods.md index 97644620..500d4658 100644 --- a/projects/ledger-app/0001-token-admin-methods.md +++ b/projects/ledger-app/0001-token-admin-methods.md @@ -208,4 +208,12 @@ This UX challenge should be addressed in its own design. We currently only support P2PKH scripts and we are adding support for data scripts (under strict conditions). The difficulty with P2SH is the lack of the `redeemScript` which is required to derive addresses, since we would require the multisig configuration to be loaded in the app and we do not have support for this yet. Without address derivation we cannot check that any addresses are from our wallet (making change output checks fail). -Also, we do not have permission to derive in the Hathor multisig path derivation (see [here](https://github.com/HathorNetwork/hathor-wallet-lib/blob/v1.4.0/src/constants.ts#L267)) so this would also need to change. \ No newline at end of file +Also, we do not have permission to derive in the Hathor multisig path derivation (see [here](https://github.com/HathorNetwork/hathor-wallet-lib/blob/v1.4.0/src/constants.ts#L267)) so this would also need to change. + +# Task breakdown + +- Parse version separate from signalBits and verify signalBits is `0x00` (0.5 dev day) +- Parse authorities from token_data when parsing outputs (1 dev day) +- Create new screen for authority output confirmation (2 dev days) +- Create new command to allow token and NFT creation (3 dev days) +- New integration tests for new commands and outputs (2 dev days) \ No newline at end of file From 84b9372555feb8760467f0826290b12b1fb385e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Carneiro?= Date: Fri, 19 Apr 2024 17:21:22 -0300 Subject: [PATCH 6/8] chore: review observations --- projects/ledger-app/0001-token-admin-methods.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/projects/ledger-app/0001-token-admin-methods.md b/projects/ledger-app/0001-token-admin-methods.md index 500d4658..8f40460b 100644 --- a/projects/ledger-app/0001-token-admin-methods.md +++ b/projects/ledger-app/0001-token-admin-methods.md @@ -35,7 +35,7 @@ To allow admin operations we will introduce new concepts into the app that were --- The `token_data` of an output is a byte with the information: -- Weather the token is a custom token or HTR +- Whether the token is a custom token or HTR - Which custom token it is (index on the token array) - If the output is an authority output (which authority will be encoded on the `value`) Currently on the app we ignore the last piece of information since we do not allow authority outputs. @@ -81,7 +81,7 @@ Version byte needs to be refactored to a 1 byte field and support for `signalBit ### Mint, Melt, Delegate and Destroy Sending a mint, melt, delegate or destroy transactions should use the same protocol as sending normal transactions. -Mint and Melt involve destroying/creating custom token outputs and since the Ledger app does not have access to the input data it can only see the outputs, which means that a mint operation is a transaction with custom tokens outputs (possibly HTR outputs for change) and authority outputs, same for melt operations. +Mint and Melt involve destroying/creating custom token outputs and since the Ledger app does not have access to the input data it can only see the outputs, which means that a mint operation is a transaction with custom tokens outputs (possibly HTR outputs for change) and possibly authority outputs, same for melt operations. Delegate is simpler since its actually just sending an authority output. @@ -206,7 +206,7 @@ This UX challenge should be addressed in its own design. ### P2SH We currently only support P2PKH scripts and we are adding support for data scripts (under strict conditions). -The difficulty with P2SH is the lack of the `redeemScript` which is required to derive addresses, since we would require the multisig configuration to be loaded in the app and we do not have support for this yet. +Identifying P2SH addresses in the transaction is possible but the difficulty of generating P2SH addresses has some is the lack of the `redeemScript` which is required to derive the address, since we would require the multisig configuration to be loaded in the app and we do not have support for this yet. Without address derivation we cannot check that any addresses are from our wallet (making change output checks fail). Also, we do not have permission to derive in the Hathor multisig path derivation (see [here](https://github.com/HathorNetwork/hathor-wallet-lib/blob/v1.4.0/src/constants.ts#L267)) so this would also need to change. From 34120c515a2ea81a85f101eb381f8f2d74bf7e29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Carneiro?= Date: Mon, 6 May 2024 13:21:07 -0300 Subject: [PATCH 7/8] chore: remove isNFT suggestion --- .../ledger-app/0001-token-admin-methods.md | 77 +++++++++---------- 1 file changed, 37 insertions(+), 40 deletions(-) diff --git a/projects/ledger-app/0001-token-admin-methods.md b/projects/ledger-app/0001-token-admin-methods.md index 8f40460b..fffea229 100644 --- a/projects/ledger-app/0001-token-admin-methods.md +++ b/projects/ledger-app/0001-token-admin-methods.md @@ -3,6 +3,7 @@ Support for admin operations for custom tokens on the Ledger app. This includes: + - Creating tokens and NFTs. - Minting new tokens and NFTs. - Melting new tokens and NFTs. @@ -18,40 +19,37 @@ This design is to allow users who use Ledger, a hardware cold wallet, to fully m # Guide-level explanation [guide-level-explanation]: #guide-level-explanation -> [!WARNING] Request for comment -> This whole paragraph below can change if we really want to, we just need to add an `isNFT` boolean in the token data for the user to confirm, so in all transactions the Ledger will be informed that the token is an NFT with the user express consent during token signing -> This makes a breaking change in the wallet dekstop since we will change an existing command. -> We can avoid this command change by using the P1-P2 to create a SEND_TOKEN_DATA v2 where we have the `isNFT` and keep the v1 unchamged. - Before entering the operations, we should address the differences between custom tokens and NFTs. In Hathor, NFTs are custom tokens, where the creation transaction (which is the uid of the NFT) has the NFT data associated which is usually an IPFS link to the NFT document (image, pdf, video, etc.). After the creation the operations and transactions involving NFT are exactly the same regarding the protocol, but the balance is shown differently to the user, where the custom token unit is worth 0,01 the NFT is always integer, so it is 1. Both of these are the same value on the transaction it is just shown different on the UI. Since the Ledger cannot check if a token is an NFT or not, we should disregard any changes and treat both as custom tokens (which is valid if we follow the protocol). -## Transaction parsing +## Transaction parsing updates To allow admin operations we will introduce new concepts into the app that were being ignored or not treated. --- The `token_data` of an output is a byte with the information: + - Whether the token is a custom token or HTR -- Which custom token it is (index on the token array) +- Which custom token it is (index on the token array) - If the output is an authority output (which authority will be encoded on the `value`) -Currently on the app we ignore the last piece of information since we do not allow authority outputs. -The way we do it is by throwing an error of the `token_data & TOKEN_DATA_AUTHORITY_MASK` is not 0. -To allow authority outputs we just need to change this and create a special screen for authority outputs when the user is confirming them. + - Currently on the app we ignore the last piece of information since we do not allow authority outputs. The way we do it is by throwing an error if `token_data & TOKEN_DATA_AUTHORITY_MASK` is not 0. To allow authority outputs we just need to change this and create a special screen for authority outputs when the user is confirming them. + Usually the output confirmation screen involves 4 steps: + - Address step (with pagination) - Amount step (Showing as `ABC 0.01` or `HTR 0.01`, with pagination for big values) - Approve step - Reject step When confirming an authority output we should show: + - Address step (with pagination) - Type step (`mint authority` or `melt authority`) - - Other authorities will throw. - - We will not allow joined authority, for instance `mint+melt authority` (i.e. `value = 3`) + - Other authorities will throw. + - We will not allow joined authority, for instance `mint+melt authority` (i.e. `value = 3`) - Approve step - Reject step @@ -71,7 +69,7 @@ When creating a custom token or NFT the transaction information also includes th --- -> [!NOTE] Version field has been split into `signalBits` and `version` +> NOTE: Version field has been split into `signalBits` and `version` > The version field of the transaction was 2 bytes (first byte was reserved for later use) and since Ledger app version 1.1.0 the version field has become a 1 byte field and the first byte became the `signalBits` Version byte needs to be refactored to a 1 byte field and support for `signalBits` will not be added in this design so we will only allow `0x00` as `signalBits` for now. @@ -91,21 +89,24 @@ Granted that the token array will have the token uid of the authority being dest ### Create token Token creation transactions are special for a number of reasons: + 1. Transaction version byte is `0x03` 2. Extra fields at the end of the transaction, namely `token_info_version`, `name` and `symbol` 3. There will be an output with the created token, the token uid will not be on the tokens array since it does not exist yet - 1. Token uid is the transaction id of the created transaction, which is created during mining. + 1. Token uid is the transaction id of the created transaction, which is created during mining. To deal with these we will create a new command to load the token data of the token being created. We will use the existing `SEND_TOKEN_DATA` for instruction code `0x08` and use the `P1` field to indicate which data is being sent. +The first command in the table below is the existing `SEND_TOKEN_DATA` command, we will use `P1` of the APDU protocol to switch to the correct handler. | Description | P1 | P2 | CData | | ------------------------------------------- | ---- | ---- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | Send token data
for signing transactions | 0x00 | 0x00 | `version (1)` \|\|
`uid (32)` \|\|
`symbol_len (1)` \|\|
`symbol (symbol_len)` \|\|
`name_len (1)` \|\|
`name (name_len)` \|\|
`signature (32)` | | Send token data
for creating a token | 0x01 | 0x00 | `version (1)` \|\|
`symbol_len (1)` \|\|
`symbol (symbol_len)` \|\|
`name_len (1)` \|\|
`name (name_len)` | -| Send token data
for creating an NFT | 0x01 | 0x01 | `version (1)` \|\|
`symbol_len (1)` \|\|
`symbol (symbol_len)` \|\|
`name_len (1)` \|\|
`name (name_len)` \|\|
`data_len (1)` \|\|
`data (data_len)` | +| Send token data
for creating an NFT | 0x02 | 0x00 | `version (1)` \|\|
`symbol_len (1)` \|\|
`symbol (symbol_len)` \|\|
`name_len (1)` \|\|
`name (name_len)` \|\|
`data_len (1)` \|\|
`data (data_len)` | The new commands will require a new flow for user confirmation + 1. Screen showing the user this is a "Create token" data. 2. Screen with the symbol 3. Screen with the name @@ -148,62 +149,58 @@ typedef struct { --- -During the transaction processing we use the method `receive_data` which internally calls `_decode_elements` untill we reach a fully formed output (which triggers a user confirmation) or we reach the end of the buffer (which returns a request for more data) when we parse all outputs (as defined in the initial portion of the wallet) and there is no more data to parse we start the UI step to request the user for a final tx confirmation. +During the transaction processing we use the method `receive_data` which internally calls `_decode_elements` untill we reach a fully formed output (which triggers a user confirmation) or we reach the end of the buffer (which returns a request for more data) when we parse all outputs (as defined in the initial portion of the wallet) and there is no more data to parse we start the UI step to request the user for a final tx confirmation. To add support for our new transactions we require some changes on output confirmation. -### Output confirmation +## Output confirmation The method `ui_display_tx_outputs` prepares the outputs (address base58 encoding, value formatting for UI, etc.) and shows the screens with the output information to the user (with `ux_display_tx_output_flow`). To adjust this we need to check if the output is a P2PKH output, a data output or an authority output, each output type have different data for the user to check so it makes sense that each has its own flow (i.e. screens that display information and ask confirmation or rejection). - normal P2PKH outputs - - `pubkey_hash` field of the output should not be empty. - - `token_data` should indicate that this **IS NOT** an authority output. - - This is the current supported format. - - For a token creation, we can check if this is the token being created and use the loaded symbol for the amount. + - `pubkey_hash` field of the output should not be empty. + - `token_data` should indicate that this **IS NOT** an authority output. + - This is the current supported format. + - For a token creation, we can check if this is the token being created and use the loaded symbol for the amount. - authority P2PKH outputs - - `pubkey_hash` field of the output should not be empty. - - `token_data` should indicate that this **IS** an authority output. - - Should confirm the address and authority of the output. - - The authority step should have the title "Authority" and text either "Mint \" or "Melt \" depending on the type of authority. + - `pubkey_hash` field of the output should not be empty. + - `token_data` should indicate that this **IS** an authority output. + - Should confirm the address and authority of the output. + - The authority step should have the title "Authority" and text either "Mint \" or "Melt \" depending on the type of authority. - Data outputs - - Should only exist if the transaction version byte is `0x03` - - `data` should be ascii safe - - `value` should always be 1 - - No need for confirmation, we just need to check that the data matches the user confirmed data. + - Should only exist if the transaction version byte is `0x03` + - `data` should be ascii safe + - `value` should always be 1 + - No need for confirmation, we just need to check that the data matches the user confirmed data. -### Transaction parsing +## Transaction parsing The current process to parse the transaction will parse the 2 version bytes then the length of tokens, intputs and outputs (1 byte each) after this we parse the token uids, inputs and outputs as the length bytes described. For "create token transactions" (identified by version byte `0x03`) we have some extra fields at the end of the transaction, these fields should already be confirmed by the user so we just need to make sure the transaction information matches the user confirmed data. + # Rationale and alternatives [rationale-and-alternatives]: #rationale-and-alternatives -### Separate create tx command +## Separate create tx command We could create a separate command for create token transactions, but the transaction parsing methods are very complex and duplicating this code is not desirable. -### Do not create a new `SEND_TOKEN_DATA` command +## Do not create a new `SEND_TOKEN_DATA` command Instead of the new command to load the token data being created we could just confirm the data when they appear on the transaction parsing, this would create an issue for outputs containing the token being created since during parsing the symbol is unknown (since it is the last piece of information) so we would need to show the amount as "Amount created" or similar (same with authorities) to signify that the output is of the created token. -# Unresolved questions -[unresolved-questions]: #unresolved-questions - - - # Future possibilities [future-possibilities]: #future-possibilities -### Nano contract +## Nano contract For signing we just require an extra signature for the caller that can be called during the usual process. The issue is that we need to parse the nano contract data and add a confirmation step for these. Since the arguments can be very technical the app UX will not be very user friendly. This UX challenge should be addressed in its own design. -### P2SH +## P2SH We currently only support P2PKH scripts and we are adding support for data scripts (under strict conditions). Identifying P2SH addresses in the transaction is possible but the difficulty of generating P2SH addresses has some is the lack of the `redeemScript` which is required to derive the address, since we would require the multisig configuration to be loaded in the app and we do not have support for this yet. @@ -216,4 +213,4 @@ Also, we do not have permission to derive in the Hathor multisig path derivation - Parse authorities from token_data when parsing outputs (1 dev day) - Create new screen for authority output confirmation (2 dev days) - Create new command to allow token and NFT creation (3 dev days) -- New integration tests for new commands and outputs (2 dev days) \ No newline at end of file +- New integration tests for new commands and outputs (2 dev days) From 9104b73ee7ac11ff4a6245de63f77e814006e9af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Carneiro?= Date: Tue, 4 Jun 2024 18:20:32 -0300 Subject: [PATCH 8/8] doc: update design with recent changes --- .../ledger-app/0001-token-admin-methods.md | 62 +++++++++++++------ 1 file changed, 42 insertions(+), 20 deletions(-) diff --git a/projects/ledger-app/0001-token-admin-methods.md b/projects/ledger-app/0001-token-admin-methods.md index fffea229..2eb5c106 100644 --- a/projects/ledger-app/0001-token-admin-methods.md +++ b/projects/ledger-app/0001-token-admin-methods.md @@ -91,21 +91,20 @@ Granted that the token array will have the token uid of the authority being dest Token creation transactions are special for a number of reasons: 1. Transaction version byte is `0x03` -2. Extra fields at the end of the transaction, namely `token_info_version`, `name` and `symbol` -3. There will be an output with the created token, the token uid will not be on the tokens array since it does not exist yet +2. There is no token array on the transaction, since there can only be HTR and the token being created. +3. Extra fields at the end of the transaction, namely `token_info_version`, `name` and `symbol` +4. There will be an output with the created token, the token uid will not be on the tokens array since it does not exist yet 1. Token uid is the transaction id of the created transaction, which is created during mining. To deal with these we will create a new command to load the token data of the token being created. -We will use the existing `SEND_TOKEN_DATA` for instruction code `0x08` and use the `P1` field to indicate which data is being sent. -The first command in the table below is the existing `SEND_TOKEN_DATA` command, we will use `P1` of the APDU protocol to switch to the correct handler. +We will create the `SEND_CREATE_TOKEN_DATA` for instruction code `0x0B` and use the `P1` field to indicate which data is being sent. | Description | P1 | P2 | CData | | ------------------------------------------- | ---- | ---- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| Send token data
for signing transactions | 0x00 | 0x00 | `version (1)` \|\|
`uid (32)` \|\|
`symbol_len (1)` \|\|
`symbol (symbol_len)` \|\|
`name_len (1)` \|\|
`name (name_len)` \|\|
`signature (32)` | -| Send token data
for creating a token | 0x01 | 0x00 | `version (1)` \|\|
`symbol_len (1)` \|\|
`symbol (symbol_len)` \|\|
`name_len (1)` \|\|
`name (name_len)` | -| Send token data
for creating an NFT | 0x02 | 0x00 | `version (1)` \|\|
`symbol_len (1)` \|\|
`symbol (symbol_len)` \|\|
`name_len (1)` \|\|
`name (name_len)` \|\|
`data_len (1)` \|\|
`data (data_len)` | +| Send token data
for creating a token | 0x00 | 0x00 | `version (1)` \|\|
`symbol_len (1)` \|\|
`symbol (symbol_len)` \|\|
`name_len (1)` \|\|
`name (name_len)` | +| Send token data
for creating an NFT | 0x01 | 0x00 | `version (1)` \|\|
`name_len (1)` \|\|
`name (name_len)` \|\|
`symbol_len (1)` \|\|
`symbol (symbol_len)` \|\|
`data_len (1)` \|\|
`data (data_len)` | -The new commands will require a new flow for user confirmation +The new command will require a new flow for user confirmation. 1. Screen showing the user this is a "Create token" data. 2. Screen with the symbol @@ -133,17 +132,35 @@ bool is_melt_authority(tx_output_t output) { } ``` -For data outputs we will introduce a new `data` field which will hold the actual data script. -We should fail if the data script is not ascii. +For data outputs, we will create an output type and the actual data will be saved on the global context, this is to be economic with in-memory data. +Also, data extracted from a data script should be ascii encoded, if not we should fail signing the transaction. ```c +// src/transaction/types.h + +/** + * Script types + */ +typedef enum { + SCRIPT_UNKNOWN = 0, + SCRIPT_P2PKH = 1, + SCRIPT_P2SH = 2, + SCRIPT_DATA = 3, +} script_type_t; + +/** + * Output script info + */ +typedef struct { + script_type_t type; + uint8_t hash[PUBKEY_HASH_LEN]; // hash160 of pubkey +} output_script_info_t; + typedef struct {     uint8_t index;     uint64_t value;     uint8_t token_data; -    uint8_t pubkey_hash[PUBKEY_HASH_LEN]; -    // New data field -    uint8_t data[MAX_DATA_SCRIPT_LEN] +    output_script_info_t script; } tx_output_t; ``` @@ -158,26 +175,26 @@ To add support for our new transactions we require some changes on output confir The method `ui_display_tx_outputs` prepares the outputs (address base58 encoding, value formatting for UI, etc.) and shows the screens with the output information to the user (with `ux_display_tx_output_flow`). To adjust this we need to check if the output is a P2PKH output, a data output or an authority output, each output type have different data for the user to check so it makes sense that each has its own flow (i.e. screens that display information and ask confirmation or rejection). -- normal P2PKH outputs - - `pubkey_hash` field of the output should not be empty. +- normal P2PKH/P2SH outputs + - `output.script.hash` field of the output should not be empty. - `token_data` should indicate that this **IS NOT** an authority output. - This is the current supported format. - For a token creation, we can check if this is the token being created and use the loaded symbol for the amount. -- authority P2PKH outputs - - `pubkey_hash` field of the output should not be empty. +- authority P2PKH/P2SH outputs + - `output.script.hash` field of the output should not be empty. - `token_data` should indicate that this **IS** an authority output. - Should confirm the address and authority of the output. - - The authority step should have the title "Authority" and text either "Mint \" or "Melt \" depending on the type of authority. + - The authority step should have the title "Authority" and text either "\ Mint" or "\ Melt" depending on the type of authority. - Data outputs - Should only exist if the transaction version byte is `0x03` - `data` should be ascii safe - - `value` should always be 1 + - `value` should always be 1 (0.01 HTR) - No need for confirmation, we just need to check that the data matches the user confirmed data. ## Transaction parsing The current process to parse the transaction will parse the 2 version bytes then the length of tokens, intputs and outputs (1 byte each) after this we parse the token uids, inputs and outputs as the length bytes described. -For "create token transactions" (identified by version byte `0x03`) we have some extra fields at the end of the transaction, these fields should already be confirmed by the user so we just need to make sure the transaction information matches the user confirmed data. +For "create token transactions" (identified by version byte `0x03`) we need to skip the length of tokens and parse some extra fields at the end of the transaction, these fields should already be confirmed by the user so we just need to make sure the transaction information matches the user confirmed data. # Rationale and alternatives [rationale-and-alternatives]: #rationale-and-alternatives @@ -190,6 +207,11 @@ We could create a separate command for create token transactions, but the transa Instead of the new command to load the token data being created we could just confirm the data when they appear on the transaction parsing, this would create an issue for outputs containing the token being created since during parsing the symbol is unknown (since it is the last piece of information) so we would need to show the amount as "Amount created" or similar (same with authorities) to signify that the output is of the created token. +## Keep a buffer for nft data in the output type + +The maximum allowed length for the data in the desktop wallet is 150 characters, if we have one for each token it would add 150 * number of tokens on global context (10), this would add 1500 bytes to the global context, but we only have 536 bytes available to the global context, any more than this will fail the build of the app. +Using 1 buffer on the global context makes the create token transaction possible and adds only 300 bytes to the global context (150 for the context and 150 for the static variable for screen display). + # Future possibilities [future-possibilities]: #future-possibilities