Feature/mintlayer pk #1

OBorce · 2024-05-15T09:36:11Z

Add new messages in the proto files for Mintlayer coin support, and handlers.

azarovh · 2024-08-06T09:55:45Z

I wouldn't merge this to master but rather created a separate branch like mintlayer-master and merge features there. This way we can keep it clean and isolated and when we decide to update trezor version we will pull to master and rebase our master. Otherwise it could become a mess very quickly. Makes sense?

One thing to consider is that CI might not work for "our" master.

ImplOfAnImpl

As I can see, there are still many FIXMEs and commented lines, so it seems like this is still work-in-progress.

core/embed/extmod/modtrezorcrypto/modtrezorcrypto-bip32.h

core/embed/extmod/modtrezormintlayer/modtrezormintlayer-utils.h

rust/trezor-client/src/error.rs

core/src/apps/bitcoin/sign_tx/layout.py

core/src/apps/workflow_handlers.py

core/src/trezor/crypto/bech32.py

ImplOfAnImpl · 2024-08-07T14:30:30Z

core/src/trezor/crypto/bech32.py

+def reverse_convertbits(
+    data: Sequence[int], frombits: int, tobits: int
+) -> list[int]:
+    acc = 0
+    bits = 0
+    ret = []
+    maxv = (1 << frombits) - 1
+    for value in data:
+        if value < 0 or (value >> tobits):
+            raise ValueError  # input value does not match `tobits` size
+        acc = ((acc << tobits) | value) & ((1 << (frombits + tobits)) - 1)
+        bits += tobits
+        while bits >= frombits:
+            bits -= frombits
+            ret.append((acc >> bits) & maxv)
+    return ret


Why is this needed? As I can see, the existing convertbits can convert in both directions, i.e. 8 to 5 and 5 to 8. Am I missing something?

it is not working correctly in the other way.

core/src/trezor/crypto/bech32.py

ImplOfAnImpl

This review is also quite superficial, because I still don't fully understand what calls what.

P.S. are there any tests for the new code?

ImplOfAnImpl · 2024-09-20T15:37:12Z

common/protob/messages-mintlayer.proto

+ * @embed
+ */
+message MintlayerTransferTxOutput {
+    required string address = 1;                    // destination address in Base58 encoding; script_type must be PAYTOADDRESS


Here and in other places - there is no "script_type", so "script_type must be PAYTOADDRESS" makes no sense

ImplOfAnImpl · 2024-09-20T16:07:59Z

common/protob/messages-mintlayer.proto

+ * @embed
+ */
+message MintlayerLockThenTransferTxOutput {
+    optional string address = 1;                    // destination address in Base58 encoding; script_type must be PAYTOADDRESS


Why is this optional?

ImplOfAnImpl · 2024-09-20T16:10:31Z

common/protob/messages-mintlayer.proto

+ * @embed
+ */
+message MintlayerDelegateStakingTxOutput {
+    required bytes amount = 1;               // the amount to stake


the amount to ~~stake~~ delegate

ImplOfAnImpl · 2024-09-20T16:11:13Z

common/protob/messages-mintlayer.proto

+    required MintlayerTokenTotalSupplyType type = 1;    // the amount to stake
+    optional bytes fixed_amount = 2;                    // the amount to stake


wrong comments

ImplOfAnImpl · 2024-09-20T16:12:43Z

common/protob/messages-mintlayer.proto

+    required bytes pool_id = 1;                   // the pool id
+    required bytes pledge = 2;                    // the pledge amount
+    required string staker = 3;                    // the staker destination address
+    required string vrf_public_key = 4;            // the VRF public key address
+    required string decommission_key = 5;          // the decommission key
+    required uint32 margin_ratio_per_thousand = 6;// the margin ratio per thousand
+    required bytes cost_per_block = 7;            // the cost per block


Plz align the comments like they do it in their original files. Same in other places.

ImplOfAnImpl · 2024-09-23T14:54:23Z

core/embed/rust/src/mintlayer/mod.rs

+#[derive(Debug, Clone, PartialEq, Eq, Encode, Decode)]
+struct StakePoolData {


I wonder whether we should consider factoring out basic types like these from the main repo into a smaller subrepo to be able to re-use them in situations like this.

ImplOfAnImpl · 2024-09-23T15:03:05Z

core/embed/extmod/modtrezormintlayer/modtrezormintlayer.h

+  vstr_t pkh = {0};
+  vstr_init_len(&pkh, arr.len_or_err.len);
+  int i = 0;
+  for (; i < arr.len_or_err.len; i++) {
+    ((uint8_t *)pkh.buf)[i] = (uint8_t)arr.data[i];
+  }


Plz use a miningful name instead of "pkh" (it's not a public key hash here).

Use memcpy instead of looping.

Same in other places.

ImplOfAnImpl · 2024-09-23T15:12:37Z

core/embed/extmod/modtrezormintlayer/modtrezormintlayer.h

+  mp_get_buffer_raise(delegation_id, &hash, MP_BUFFER_READ);
+  if (hash.len != 32) {
+    printf("invalid hash len: %ld", (long int)hash.len);
+    mp_raise_ValueError("Invalid hash");
+  }


Let's not repeat boilerplate over and over. E.g. this can be moved into a function mp_get_hash_raise, which would call mp_get_buffer_raise and then check the length.

ImplOfAnImpl · 2024-09-23T15:21:29Z

core/embed/extmod/modtrezormintlayer/modtrezormintlayer.h

+  handle_err(&arr);
+
+  vstr_t pkh = {0};
+  vstr_init_len(&pkh, arr.len_or_err.len);
+  int i = 0;
+  for (; i < arr.len_or_err.len; i++) {
+    ((uint8_t *)pkh.buf)[i] = (uint8_t)arr.data[i];
+  }
+
+  return mp_obj_new_str_from_vstr(&mp_type_bytes, &pkh);


This code is repeated, let's put it in a function. E.g. something like

mp_obj_t new_bytes_obj_from_byte_array_raise(const ByteArray *arr) { assert(arr); handle_err(arr); ... }

ImplOfAnImpl · 2024-09-23T15:32:37Z

common/protob/messages-mintlayer.proto

+/**
+* Type of information required by transaction signing process
+*/
+enum MintlayerTokenTotalSupplyType {
+    FIXED = 0;
+    LOCKABLE = 1;
+    UNLIMITED = 2;
+}
+


Also wrong comment?

OBorce force-pushed the feature/mintlayer-pk branch from bddbd25 to 967640b Compare July 16, 2024 09:08

OBorce force-pushed the feature/mintlayer-pk branch from 5abd648 to 798be63 Compare August 6, 2024 09:54

ImplOfAnImpl reviewed Aug 7, 2024

View reviewed changes

OBorce force-pushed the feature/mintlayer-pk branch 15 times, most recently from a7ca54f to 941d641 Compare September 12, 2024 15:26

OBorce added 11 commits September 12, 2024 17:47

WIP: add mintlayer messages for public key

d0703f3

add mintlayer sing tx messages

a031759

WIP: more messages

79d607b

correct hash and signatures

8813b66

remove dead code copied from bitcoin

a6174a0

Fix compilation for trezor device with new mintlayer code

c1d676f

Add support for all Mintlayer inputs and outputs

da5559e

fix compilation with rust nightly

2ad95c5

update mintlayer messages

d9a756b

remove unused imports

bb4f8d0

remove unused argument

440a278

OBorce added 7 commits September 12, 2024 17:55

handle token decimals and ticker on UI

705d773

add support for signing ML HTLC outputs

dd19b37

Handle errors in Mintlayer rust code instead of panicing

5effea9

Clean up code Mintlayer

f9a5baa

Regenerate with proto 3.19.6

3d5482d

add Mintlayer ChangeTokenMetadataUri command input

f56852e

fix Python typings Mintlayer

653acc9

OBorce force-pushed the feature/mintlayer-pk branch from 941d641 to dd3f1e8 Compare September 12, 2024 16:29

OBorce changed the base branch from master to main September 12, 2024 16:32

OBorce force-pushed the feature/mintlayer-pk branch 5 times, most recently from 2d02cde to 86275f8 Compare September 15, 2024 17:44

fix confirm total after rebase

2a0e5ec

OBorce force-pushed the feature/mintlayer-pk branch from 86275f8 to 2a0e5ec Compare September 15, 2024 18:10

add changelog for pipeline Mintlayer

4e26426

OBorce force-pushed the feature/mintlayer-pk branch 5 times, most recently from 79e901f to 180e245 Compare September 17, 2024 18:36

Add support for Mintlayer orders

0bf0d4a

OBorce force-pushed the feature/mintlayer-pk branch from 180e245 to 0bf0d4a Compare September 17, 2024 18:43

fix errors in Mintlayer layout

a3bda3f

OBorce force-pushed the feature/mintlayer-pk branch from 3529ba2 to a3bda3f Compare September 19, 2024 09:53

ImplOfAnImpl reviewed Sep 23, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/mintlayer pk #1

Feature/mintlayer pk #1

OBorce commented May 15, 2024 •

edited

Loading

azarovh commented Aug 6, 2024 •

edited

Loading

ImplOfAnImpl left a comment

ImplOfAnImpl Aug 7, 2024

OBorce Sep 10, 2024

ImplOfAnImpl left a comment

ImplOfAnImpl Sep 20, 2024

ImplOfAnImpl Sep 20, 2024

ImplOfAnImpl Sep 20, 2024

ImplOfAnImpl Sep 20, 2024

ImplOfAnImpl Sep 20, 2024

ImplOfAnImpl Sep 23, 2024

ImplOfAnImpl Sep 23, 2024

ImplOfAnImpl Sep 23, 2024

ImplOfAnImpl Sep 23, 2024

ImplOfAnImpl Sep 23, 2024

		required MintlayerTokenTotalSupplyType type = 1; // the amount to stake
		optional bytes fixed_amount = 2; // the amount to stake

		#[derive(Debug, Clone, PartialEq, Eq, Encode, Decode)]
		struct StakePoolData {

Feature/mintlayer pk #1

Are you sure you want to change the base?

Feature/mintlayer pk #1

Conversation

OBorce commented May 15, 2024 • edited Loading

azarovh commented Aug 6, 2024 • edited Loading

ImplOfAnImpl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ImplOfAnImpl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OBorce commented May 15, 2024 •

edited

Loading

azarovh commented Aug 6, 2024 •

edited

Loading