feat!: add payment id #6340

SWvheerden · 2024-05-16T11:18:24Z

Description

Add payment id as buckets into encrypted data field

Motivation and Context

Allows wallets to send encrypted data via the block chain to other wallets

github-actions · 2024-05-16T11:26:11Z

Test Results (CI)

3 files 120 suites 38m 59s ⏱️
1 295 tests 1 295 ✅ 0 💤 0 ❌
3 877 runs 3 877 ✅ 0 💤 0 ❌

Results for commit 7ad53c8.

♻️ This comment has been updated with latest results.

github-actions · 2024-05-16T11:26:41Z

Test Results (Integration tests)

2 files + 2 1 errors 9 suites +9 10m 46s ⏱️ + 10m 46s
18 tests +18 13 ✅ +13 0 💤 ±0 5 ❌ + 5
27 runs +27 14 ✅ +14 0 💤 ±0 13 ❌ +13

For more details on these parsing errors and failures, see this check.

Results for commit 7ad53c8. ± Comparison against base commit 0fd2efe.

♻️ This comment has been updated with latest results.

hansieodendaal

Looking good, some comments relating to the interpretation of the requirement for a payment ID.

hansieodendaal · 2024-05-20T06:00:24Z

applications/minotari_console_wallet/src/ui/state/app_state.rs

+        let payment_id_u64: u64 = payment_id_hex
+            .parse::<u64>()
+            .map_err(|_| UiError::HexError("Could not convert payment_id to bytes".to_string()))?;
+        let payment_id = PaymentId::U64(payment_id_u64);


We should allow 256 bytes, not only 8.

This is just UI support for now, in CW to only support u64

hansieodendaal · 2024-05-20T06:01:04Z

applications/minotari_console_wallet/src/ui/state/app_state.rs

+        let payment_id_u64: u64 = payment_id_hex
+            .parse::<u64>()
+            .map_err(|_| UiError::HexError("Could not convert payment_id to bytes".to_string()))?;
+        let payment_id = PaymentId::U64(payment_id_u64);


hansieodendaal · 2024-05-20T06:10:35Z

base_layer/core/src/transactions/transaction_components/encrypted_data.rs

+pub enum PaymentId {
+    Zero,
+    U32(u32),
+    U64(u64),
+    U256(U256),
+}


I think the brackets are a good idea instead of a perfect variable length.

I propose these brackets:

8 bytes

16 bytes

32 bytes

16-byte increments up to 256 bytes (48, 64, 80, ..., 256)

hansieodendaal · 2024-05-20T06:16:00Z

base_layer/core/src/consensus/consensus_constants.rs

@@ -397,6 +404,7 @@ impl ConsensusConstants {
            faucet_value: 0.into(),
            transaction_weight: TransactionWeight::latest(),
            max_script_byte_size: 2048,
+            max_encrypted_data_byte_size: 32, // U256 size


This should be the current EncryptedData::data: [u8; SIZE_TOTAL] plus 256 bytes,

I was debating about making it max_encrypted_data_byte_size or max_payment_id_size
Its currently max payment id in effect, but not name

hansieodendaal · 2024-05-20T06:16:21Z

base_layer/core/src/consensus/consensus_constants.rs

@@ -461,6 +469,7 @@ impl ConsensusConstants {
            faucet_value: 0.into(), // IGOR_FAUCET_VALUE.into(),
            transaction_weight: TransactionWeight::v1(),
            max_script_byte_size: 2048,
+            max_encrypted_data_byte_size: 32, // U256 size


fix test fix test fix

applications/minotari_app_grpc/proto/wallet.proto

applications/minotari_console_wallet/src/automation/commands.rs

applications/minotari_console_wallet/src/grpc/wallet_grpc_server.rs

stringhandler · 2024-06-03T08:28:51Z

applications/minotari_console_wallet/src/ui/components/send_tab.rs

@@ -29,6 +29,7 @@ pub struct SendTab {
    send_input_mode: SendInputMode,
    show_contacts: bool,
    to_field: String,
+    payment_id_field: String,


it might be good in future to include a drop down for "Custom Text", "Custom binary", "Sender Address"

as well as some other options in future. Exchanges might want a specific field in here.

stringhandler · 2024-06-03T08:38:05Z

base_layer/core/src/transactions/transaction_components/encrypted_data.rs

-        let mut bytes = Zeroizing::new([0u8; SIZE_VALUE + SIZE_MASK]);
-        bytes.clone_from_slice(&encrypted_data.as_bytes()[SIZE_NONCE..SIZE_NONCE + SIZE_VALUE + SIZE_MASK]);
-        let tag = Tag::from_slice(&encrypted_data.as_bytes()[SIZE_NONCE + SIZE_VALUE + SIZE_MASK..]);
+        let tag = Tag::from_slice(&encrypted_data.as_bytes()[..SIZE_TAG]);


Watch out for buffer overrun. Possibly fuzz this with quicktest

those should all be a constant size, and there min size checks

Co-authored-by: stringhandler <stringhandler@gmail.com>

Description --- Add payment id as buckets into encrypted data field Motivation and Context --- Allows wallets to send encrypted data via the block chain to other wallets --------- Co-authored-by: stringhandler <stringhandler@gmail.com>

SWvheerden requested a review from a team as a code owner May 16, 2024 11:18

ghpbot-tari-project added P-acks_required Process - Requires more ACKs or utACKs P-reviews_required Process - Requires a review from a lead maintainer to be merged labels May 16, 2024

SWvheerden changed the title ~~[wip]feat!: add payment id~~ feat!: add payment id May 19, 2024

ghpbot-tari-project added the P-conflicts Process - The PR has merge conflicts that need to be resolved label May 19, 2024

SWvheerden force-pushed the sw_paymetnid branch from e66a857 to a4e17fd Compare May 19, 2024 12:16

ghpbot-tari-project removed the P-conflicts Process - The PR has merge conflicts that need to be resolved label May 19, 2024

hansieodendaal reviewed May 20, 2024

View reviewed changes

SWvheerden force-pushed the sw_paymetnid branch 2 times, most recently from f5c471a to c8fa3de Compare May 30, 2024 13:34

SWvheerden added 9 commits June 3, 2024 09:26

initial

cc4f0ce

add max check size

d7c3b4a

lib ffi

a9636a3

add byte size calc

463ab63

disply payment_id

b13b716

fix wallet display

51eeb14

fix tests

c22283c

fix test fix test fix

fix tests

61b48f0

fix test

2b7bc0b

SWvheerden force-pushed the sw_paymetnid branch from f1de446 to 2b7bc0b Compare June 3, 2024 07:31

stringhandler reviewed Jun 3, 2024

View reviewed changes

SWvheerden and others added 4 commits June 3, 2024 11:25

Update applications/minotari_console_wallet/src/automation/commands.rs

693d310

Co-authored-by: stringhandler <stringhandler@gmail.com>

changes

facbcf8

fix integration test

15a7382

lib changes

7ad53c8

stringhandler approved these changes Jun 3, 2024

View reviewed changes

ghpbot-tari-project removed the P-reviews_required Process - Requires a review from a lead maintainer to be merged label Jun 3, 2024

SWvheerden merged commit ec4e5e9 into tari-project:development Jun 3, 2024
14 of 16 checks passed

SWvheerden deleted the sw_paymetnid branch June 3, 2024 12:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat!: add payment id #6340

feat!: add payment id #6340

SWvheerden commented May 16, 2024

github-actions bot commented May 16, 2024 •

edited

github-actions bot commented May 16, 2024 •

edited

hansieodendaal left a comment

hansieodendaal May 20, 2024

SWvheerden May 31, 2024

hansieodendaal May 20, 2024

hansieodendaal May 20, 2024

hansieodendaal May 20, 2024

SWvheerden May 20, 2024

hansieodendaal May 20, 2024

stringhandler Jun 3, 2024

stringhandler Jun 3, 2024

stringhandler Jun 3, 2024

SWvheerden Jun 3, 2024 •

edited

feat!: add payment id #6340

feat!: add payment id #6340

Conversation

SWvheerden commented May 16, 2024

Description

Motivation and Context

github-actions bot commented May 16, 2024 • edited

Test Results (CI)

github-actions bot commented May 16, 2024 • edited

Test Results (Integration tests)

hansieodendaal 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

SWvheerden Jun 3, 2024 • edited

Choose a reason for hiding this comment

github-actions bot commented May 16, 2024 •

edited

github-actions bot commented May 16, 2024 •

edited

SWvheerden Jun 3, 2024 •

edited