From 92471dda68cb115c008d9d9dc788e4de48ddd8ca Mon Sep 17 00:00:00 2001 From: Skydev0h Date: Fri, 14 Jun 2024 18:30:35 +0300 Subject: [PATCH 1/4] Some improvements and changes to the contract 1) Internal messages can use any send_mode 2) Action list verification accounts for exotic cells 4) Removed unneccessary (and maybe even harming) shortcuts 5) Deduplicated process_signed_xxx code into single fun --- contracts/wallet_v5.fc | 124 +++++++++++++---------------------------- 1 file changed, 38 insertions(+), 86 deletions(-) diff --git a/contracts/wallet_v5.fc b/contracts/wallet_v5.fc index e8497e4..5f771c0 100644 --- a/contracts/wallet_v5.fc +++ b/contracts/wallet_v5.fc @@ -17,6 +17,8 @@ const int size::flags = 4; (slice) udict_get_or_return(cell dict, int key_len, int index) impure asm(index dict key_len) "DICTUGET" "IFNOTRET"; +(slice, int) begin_parse_xc(cell c) asm "XCTOS"; + (slice) enforce_and_remove_sign_prefix(slice body) impure asm "x{7369676E} SDBEGINS"; (slice, int) check_and_remove_extn_prefix(slice body) impure asm "x{6578746E} SDBEGINSQ"; (slice, int) check_and_remove_sint_prefix(slice body) impure asm "x{73696E74} SDBEGINSQ"; @@ -48,18 +50,18 @@ int count_trailing_ones(slice cs) asm "SDCNTTRAIL1"; slice get_last_bits(slice s, int n) asm "SDCUTLAST"; slice remove_last_bits(slice s, int n) asm "SDSKIPLAST"; -cell verify_actions(cell c5) inline { +cell verify_actions(cell c5, int is_external) inline { ;; Comment out code starting from here to disable checks (unsafe version) ;; {- - slice c5s = c5.begin_parse(); + (slice c5s, _) = c5.begin_parse_xc(); return_if(c5s.slice_empty?()); do { ;; only send_msg is allowed, set_code or reserve_currency are not c5s = c5s.enforce_and_remove_action_send_msg_prefix(); ;; enforce that send_mode has 2 bit set ;; for that load 7 bits and make sure that they end with 1 - throw_if(37, count_trailing_zeroes(c5s.preload_bits(7))); - c5s = c5s.preload_ref().begin_parse(); + throw_if(37, is_external & count_trailing_zeroes(c5s.preload_bits(7))); + (c5s, _) = c5s.preload_ref().begin_parse_xc(); } until (c5s.slice_empty?()); ;; -} return c5; @@ -68,7 +70,7 @@ cell verify_actions(cell c5) inline { ;; Dispatches already authenticated request. ;; this function is explicitly included as an inline reference - not completely inlined ;; completely inlining it causes undesirable code split and noticeable gas increase in some paths -() dispatch_complex_request(slice cs) impure inline_ref { +() dispatch_complex_request(slice cs, int is_external) impure inline_ref { ;; Recurse into extended actions until we reach standard actions while (cs~load_int(1)) { @@ -144,81 +146,18 @@ cell verify_actions(cell c5) inline { } ;; At this point we are at `action_list_basic$0 {n:#} actions:^(OutList n) = ActionList n 0;` ;; Simply set the C5 register with all pre-computed actions after verification: - set_actions(cs.preload_ref().verify_actions()); + set_actions(cs.preload_ref().verify_actions(is_external)); return (); } ;; ------------------------------------------------------------------------------------------------ ;; Verifies signed request, prevents replays and proceeds with `dispatch_request`. -() process_signed_request_from_external_message(slice full_body) impure inline { - ;; The precise order of operations here is VERY important. Any other order results in unneccessary stack shuffles. - slice signature = full_body.get_last_bits(512); - slice signed = full_body.remove_last_bits(512); - - var cs = signed.skip_bits(32); - var (subwallet_id, valid_until, msg_seqno) = (cs~load_uint(size::subwallet_id), cs~load_uint(size::valid_until), cs~load_uint(size::msg_seqno)); - - var ds = get_data().begin_parse(); - var stored_seqno = ds~load_int(size::stored_seqno); - var immutable_tail = ds; ;; stored_subwallet ~ public_key ~ extensions - var stored_subwallet = ds~load_uint(size::stored_subwallet); - var public_key = ds.preload_uint(size::public_key); - - ;; TODO: Consider moving signed into separate ref, slice_hash consumes 500 gas just like cell creation! - ;; Only such checking order results in least amount of gas - throw_unless(35, check_signature(slice_hash(signed), signature, public_key)); - ;; If public key is disabled, stored_seqno is strictly less than zero: stored_seqno < 0 - ;; However, msg_seqno is uint, therefore it can be only greater or equal to zero: msg_seqno >= 0 - ;; Thus, if public key is disabled, these two domains NEVER intersect, and additional check is not needed - throw_unless(33, msg_seqno == stored_seqno); - throw_unless(34, subwallet_id == stored_subwallet); - throw_if(36, valid_until <= now()); - - accept_message(); - - ;; Store and commit the seqno increment to prevent replays even if the subsequent requests fail. - stored_seqno = stored_seqno + 1; - set_data(begin_cell() - .store_int(stored_seqno, size::stored_seqno) - .store_slice(immutable_tail) ;; stored_subwallet ~ public_key ~ extensions - .end_cell()); - - commit(); - - if (count_leading_zeroes(cs)) { ;; starts with bit 0 - return set_actions(cs.preload_ref().verify_actions()); +() process_signed_request(slice full_body, int is_external) impure inline { + ifnot (is_external) { + ;; Additional check to make sure that there are enough bits for reading (+1 for actual actions flag) + return_if(full_body.slice_bits() < 32 + size::subwallet_id + size::valid_until + size::msg_seqno + 1 + 512); } - ;; <<<<<<<<<<---------- Simple primary cases gas evaluation ends here ---------->>>>>>>>>> - - ;; inline_ref required because otherwise it will produce undesirable JMPREF - dispatch_complex_request(cs); -} - -() recv_external(slice body) impure inline { - slice full_body = body; - ;; 0x7369676E ("sign") external message authenticated by signature - body = enforce_and_remove_sign_prefix(body); - process_signed_request_from_external_message(full_body); - return(); -} - -;; ------------------------------------------------------------------------------------------------ - -() dispatch_extension_request(slice cs, var dummy1) impure inline { - if (count_leading_zeroes(cs)) { ;; starts with bit 0 - return set_actions(cs.preload_ref().verify_actions()); - } - ;; <<<<<<<<<<---------- Simple primary cases gas evaluation ends here ---------->>>>>>>>>> - ;; - dummy1~impure_touch(); ;; DROP merged to 2DROP! - dispatch_complex_request(cs); -} - -;; Same logic as above function but with return_* instead of throw_* and additional checks to prevent bounces -() process_signed_request_from_internal_message(slice full_body) impure inline { - ;; Additional check to make sure that there are enough bits for reading (+1 for actual actions flag) - return_if(full_body.slice_bits() < 32 + size::subwallet_id + size::valid_until + size::msg_seqno + 1 + 512); ;; The precise order of operations here is VERY important. Any other order results in unneccessary stack shuffles. slice signature = full_body.get_last_bits(512); @@ -234,16 +173,22 @@ cell verify_actions(cell c5) inline { var public_key = ds.preload_uint(size::public_key); ;; Note on bouncing/nonbouncing behaviour: - ;; In principle, the wallet should not bounce incoming messages as to avoid + ;; In principle, the wallet should not bounce incoming messages as to avoid ;; returning deposits back to the sender due to opcode misinterpretation. ;; However, specifically for "gasless" transactions (signed messages relayed by a 3rd party), ;; there is a risk for the relaying party to be abused: their coins should be bounced back in case of a race condition or delays. ;; We resolve this dilemma by silently failing at the signature check (therefore ordinary deposits with arbitrary opcodes never bounce), ;; but failing with exception (therefore bouncing) after the signature check. - + ;; TODO: Consider moving signed into separate ref, slice_hash consumes 500 gas just like cell creation! - ;; Only such checking order results in least amount of gas - return_unless(check_signature(slice_hash(signed), signature, public_key)); + int signature_is_valid = check_signature(slice_hash(signed), signature, public_key); + if (is_external) { + throw_unless(35, signature_is_valid); + } else { + ifnot (signature_is_valid) { + return(); + } + } ;; If public key is disabled, stored_seqno is strictly less than zero: stored_seqno < 0 ;; However, msg_seqno is uint, therefore it can be only greater or equal to zero: msg_seqno >= 0 ;; Thus, if public key is disabled, these two domains NEVER intersect, and additional check is not needed @@ -251,6 +196,10 @@ cell verify_actions(cell c5) inline { throw_unless(34, subwallet_id == stored_subwallet); throw_if(36, valid_until <= now()); + if (is_external) { + accept_message(); + } + ;; Store and commit the seqno increment to prevent replays even if the subsequent requests fail. stored_seqno = stored_seqno + 1; set_data(begin_cell() @@ -260,15 +209,19 @@ cell verify_actions(cell c5) inline { commit(); - if (count_leading_zeroes(cs)) { ;; starts with bit 0 - return set_actions(cs.preload_ref().verify_actions()); - } - ;; <<<<<<<<<<---------- Simple primary cases gas evaluation ends here ---------->>>>>>>>>> + dispatch_complex_request(cs, is_external); +} - ;; inline_ref required because otherwise it will produce undesirable JMPREF - dispatch_complex_request(cs); +() recv_external(slice body) impure inline { + slice full_body = body; + ;; 0x7369676E ("sign") external message authenticated by signature + body = enforce_and_remove_sign_prefix(body); + process_signed_request(full_body, true); + return(); } +;; ------------------------------------------------------------------------------------------------ + () recv_internal(cell full_msg, slice body) impure inline { ;; return right away if there are no references @@ -305,8 +258,7 @@ cell verify_actions(cell c5) inline { ;; so we accept the funds silently instead of throwing an error (wallet v4 does the same). var wc = extensions.udict_get_or_return(256, packed_sender_addr); ;; kindof ifnot (success?) { return(); } - ;; auth_kind and wc are passed into dispatch_extension_request and later are dropped in batch with 3 BLKDROP - dispatch_extension_request(body, wc); ;; Special route for external address authenticated request + dispatch_complex_request(body, false); return (); } @@ -316,7 +268,7 @@ cell verify_actions(cell c5) inline { return_unless(is_sint?); ;; Process the rest of the slice just like the signed request. - process_signed_request_from_internal_message(full_body); + process_signed_request(full_body, false); return (); ;; Explicit returns escape function faster and const less gas (suddenly!) } From 7d1d14494925ce629bc0d8bbe17783decf476c7a Mon Sep 17 00:00:00 2001 From: Skydev0h Date: Fri, 14 Jun 2024 18:49:36 +0300 Subject: [PATCH 2/4] Plugins now bound to same workchain as the wallet, no packing (6) --- contracts/wallet_v5.fc | 20 ++++++++++---------- tests/utils.ts | 3 +-- tests/wallet-v5-external.spec.ts | 2 +- tests/wallet-v5-get.spec.ts | 4 ++-- tests/wallet-v5-internal.spec.ts | 2 +- wrappers/wallet-v5.ts | 2 +- 6 files changed, 16 insertions(+), 17 deletions(-) diff --git a/contracts/wallet_v5.fc b/contracts/wallet_v5.fc index 5f771c0..79f3fe6 100644 --- a/contracts/wallet_v5.fc +++ b/contracts/wallet_v5.fc @@ -32,11 +32,6 @@ const int size::flags = 4; ;; Extensible wallet contract v5 -;; Compresses 8+256-bit address into 256-bit uint by cutting off one bit from sha256. -;; This allows us to save on wrapping the address in a cell and make plugin requests cheaper. -;; This method also unpacks address hash if you pass packed hash with the original wc. -int pack_address((int, int) address) impure asm "SWAP" "INC" "XOR"; ;; hash ^ (wc+1) - ;; Stores pre-computed list of actions (mostly `action_send_msg`) in the actions register. () set_actions(cell action_list) impure asm "c5 POP"; @@ -79,7 +74,9 @@ cell verify_actions(cell c5, int is_external) inline { ;; Add/remove extensions if (is_add_ext? | is_del_ext?) { (int wc, int hash) = parse_std_addr(cs~load_msg_addr()); - int packed_addr = pack_address((wc, hash) ); + (int my_wc, _) = parse_std_addr(my_address()); + + throw_unless(45, my_wc == wc); var ds = get_data().begin_parse(); var data_bits = ds~load_bits(size::stored_seqno + size::stored_subwallet + size::public_key); @@ -88,13 +85,13 @@ cell verify_actions(cell c5, int is_external) inline { ;; Add extension if (is_add_ext?) { - (extensions, int success?) = extensions.udict_add_builder?(256, packed_addr, begin_cell().store_int(wc,8)); + (extensions, int success?) = extensions.udict_add_builder?(256, hash, begin_cell().store_int(wc,8)); throw_unless(39, success?); } else ;; Remove extension if (op == 0x5eaef4a4) ;; It can be ONLY 0x1c40db9f OR 0x5eaef4a4 here. No need for second check. { - (extensions, int success?) = extensions.udict_delete?(256, packed_addr); + (extensions, int success?) = extensions.udict_delete?(256, hash); throw_unless(40, success?); throw_if(44, null?(extensions) & (stored_seqno < 0)); } @@ -248,7 +245,10 @@ cell verify_actions(cell c5, int is_external) inline { if (is_extn?) { ;; "extn" authenticated by extension ;; Authenticate extension by its address. - int packed_sender_addr = pack_address(parse_std_addr(full_msg_slice~load_msg_addr())); ;; no PLDMSGADDR exists + (int sender_wc, int sender_hash) = parse_std_addr(full_msg_slice~load_msg_addr()); ;; no PLDMSGADDR exists + (int my_wc, _) = parse_std_addr(my_address()); + + return_unless(my_wc == sender_wc); var ds = get_data().begin_parse(); ;; It is not required to read this data here, maybe ext is doing simple transfer where those are not needed @@ -256,7 +256,7 @@ cell verify_actions(cell c5, int is_external) inline { ;; Note that some random contract may have deposited funds with this prefix, ;; so we accept the funds silently instead of throwing an error (wallet v4 does the same). - var wc = extensions.udict_get_or_return(256, packed_sender_addr); ;; kindof ifnot (success?) { return(); } + extensions.udict_get_or_return(256, sender_hash); ;; kindof ifnot (success?) { return(); } dispatch_complex_request(body, false); return (); diff --git a/tests/utils.ts b/tests/utils.ts index 8257d4b..d309bf9 100644 --- a/tests/utils.ts +++ b/tests/utils.ts @@ -5,8 +5,7 @@ export function bufferToBigInt(buffer: Buffer): bigint { } export function packAddress(address: Address) { - const wcPlus = address.workChain + 1; - return bufferToBigInt(address.hash) ^ BigInt(wcPlus); + return bufferToBigInt(address.hash); } export function validUntil(ttlMs = 1000 * 60 * 3) { diff --git a/tests/wallet-v5-external.spec.ts b/tests/wallet-v5-external.spec.ts index 3d17d1d..69e26c2 100644 --- a/tests/wallet-v5-external.spec.ts +++ b/tests/wallet-v5-external.spec.ts @@ -293,7 +293,7 @@ describe('Wallet V5 sign auth external', () => { }); it('Add two extensions and do a transfer', async () => { - const testExtension1 = Address.parse('Ef82pT4d8T7TyRsjW2BpGpGYga-lMA4JjQb4D2tc1PXMX28X'); + const testExtension1 = Address.parse('EQA2pT4d8T7TyRsjW2BpGpGYga-lMA4JjQb4D2tc1PXMX5Bf'); const testExtension2 = Address.parse('EQCgYDKqfTh7zVj9BQwOIPs4SuOhM7wnIjb6bdtM2AJf_Z9G'); const testReceiver = Address.parse('EQAvDfWFG0oYX19jwNDNBBL1rKNT9XfaGP9HyTb5nb2Eml6y'); diff --git a/tests/wallet-v5-get.spec.ts b/tests/wallet-v5-get.spec.ts index c3fac3a..6ec99d2 100644 --- a/tests/wallet-v5-get.spec.ts +++ b/tests/wallet-v5-get.spec.ts @@ -107,7 +107,7 @@ describe('Wallet V5 get methods', () => { it('Get extensions dict', async () => { const plugin1 = Address.parse('EQAvDfWFG0oYX19jwNDNBBL1rKNT9XfaGP9HyTb5nb2Eml6y'); - const plugin2 = Address.parse('Ef82pT4d8T7TyRsjW2BpGpGYga-lMA4JjQb4D2tc1PXMX28X'); + const plugin2 = Address.parse('EQA2pT4d8T7TyRsjW2BpGpGYga-lMA4JjQb4D2tc1PXMX5Bf'); const extensions: Dictionary = Dictionary.empty( Dictionary.Keys.BigUint(256), @@ -127,7 +127,7 @@ describe('Wallet V5 get methods', () => { it('Get extensions array', async () => { const plugin1 = Address.parse('EQAvDfWFG0oYX19jwNDNBBL1rKNT9XfaGP9HyTb5nb2Eml6y'); - const plugin2 = Address.parse('Ef82pT4d8T7TyRsjW2BpGpGYga-lMA4JjQb4D2tc1PXMX28X'); + const plugin2 = Address.parse('EQA2pT4d8T7TyRsjW2BpGpGYga-lMA4JjQb4D2tc1PXMX5Bf'); const extensions: Dictionary = Dictionary.empty( Dictionary.Keys.BigUint(256), diff --git a/tests/wallet-v5-internal.spec.ts b/tests/wallet-v5-internal.spec.ts index 61f79f0..6459520 100644 --- a/tests/wallet-v5-internal.spec.ts +++ b/tests/wallet-v5-internal.spec.ts @@ -266,7 +266,7 @@ describe('Wallet V5 sign auth internal', () => { }); it('Add two extensions and do a transfer', async () => { - const testExtension1 = Address.parse('Ef82pT4d8T7TyRsjW2BpGpGYga-lMA4JjQb4D2tc1PXMX28X'); + const testExtension1 = Address.parse('EQA2pT4d8T7TyRsjW2BpGpGYga-lMA4JjQb4D2tc1PXMX5Bf'); const testExtension2 = Address.parse('EQCgYDKqfTh7zVj9BQwOIPs4SuOhM7wnIjb6bdtM2AJf_Z9G'); const testReceiver = Address.parse('EQAvDfWFG0oYX19jwNDNBBL1rKNT9XfaGP9HyTb5nb2Eml6y'); diff --git a/wrappers/wallet-v5.ts b/wrappers/wallet-v5.ts index fe0c708..89c991b 100644 --- a/wrappers/wallet-v5.ts +++ b/wrappers/wallet-v5.ts @@ -232,7 +232,7 @@ export class WalletV5 implements Contract { return dict.keys().map(key => { const wc = dict.get(key)!; - const addressHex = key ^ (wc + 1n); + const addressHex = key; return Address.parseRaw(`${wc}:${addressHex.toString(16)}`); }); } From 1f9b0a3a19edfab4ae4d6bfd23eb0fea1e5fc181 Mon Sep 17 00:00:00 2001 From: Skydev0h Date: Fri, 14 Jun 2024 19:15:57 +0300 Subject: [PATCH 3/4] Moved signature_auth_disable to separate variable from seq_no sign (3) --- Specification.md | 2 +- contracts/wallet_v5.fc | 57 +++++++++++++++--------------- scripts/deployWalletV5.ts | 1 + tests/wallet-v5-extensions.spec.ts | 11 ++---- tests/wallet-v5-external.spec.ts | 13 ++++--- tests/wallet-v5-get.spec.ts | 1 + tests/wallet-v5-internal.spec.ts | 15 ++++---- types.tlb | 2 +- wrappers/wallet-v5.ts | 4 ++- 9 files changed, 52 insertions(+), 54 deletions(-) diff --git a/Specification.md b/Specification.md index ed44015..fdfef5d 100644 --- a/Specification.md +++ b/Specification.md @@ -180,7 +180,7 @@ actions$_ {m:#} {n:#} actions:(ActionList n m) = InnerRequest; Contract state: ```tl-b wallet_id$_ global_id:# wc:int8 version:(## 8) subwallet_number:# = WalletID; -contract_state$_ seqno:int33 wallet_id:WalletID public_key:(## 256) extensions_dict:(HashmapE 256 int8) = ContractState; +contract_state$_ signature_auth_disabled:(## 1) seqno:# wallet_id:WalletID public_key:(## 256) extensions_dict:(HashmapE 256 int8) = ContractState; ``` ## Source code diff --git a/contracts/wallet_v5.fc b/contracts/wallet_v5.fc index 79f3fe6..e18faf8 100644 --- a/contracts/wallet_v5.fc +++ b/contracts/wallet_v5.fc @@ -2,7 +2,8 @@ #include "imports/stdlib.fc"; -const int size::stored_seqno = 33; +const int size::signature_auth_disabled = 1; +const int size::stored_seqno = 32; const int size::stored_subwallet = 80; const int size::public_key = 256; @@ -79,8 +80,8 @@ cell verify_actions(cell c5, int is_external) inline { throw_unless(45, my_wc == wc); var ds = get_data().begin_parse(); - var data_bits = ds~load_bits(size::stored_seqno + size::stored_subwallet + size::public_key); - var stored_seqno = data_bits.preload_int(size::stored_seqno); + var data_bits = ds~load_bits(size::signature_auth_disabled + size::stored_seqno + size::stored_subwallet + size::public_key); + var signature_auth_disabled = data_bits.preload_int(size::signature_auth_disabled); var extensions = ds.preload_dict(); ;; Add extension @@ -93,7 +94,7 @@ cell verify_actions(cell c5, int is_external) inline { { (extensions, int success?) = extensions.udict_delete?(256, hash); throw_unless(40, success?); - throw_if(44, null?(extensions) & (stored_seqno < 0)); + throw_if(44, null?(extensions) & (signature_auth_disabled)); } set_data(begin_cell() @@ -104,28 +105,24 @@ cell verify_actions(cell c5, int is_external) inline { elseif (cs~check_and_remove_set_signature_auth_allowed_prefix()) { var allow? = cs~load_int(1); var ds = get_data().begin_parse(); - var stored_seqno = ds~load_int(size::stored_seqno); + var signature_auth_disabled = ds~load_int(size::signature_auth_disabled); + var stored_seqno = ds~load_uint(size::stored_seqno); var immutable_tail = ds; ;; stored_subwallet ~ public_key ~ extensions if (allow?) { ;; allow - throw_unless(43, stored_seqno < 0); - ;; Can't be disallowed with 0 because disallowing increments seqno - ;; -123 -> 123 -> 124 - stored_seqno = - stored_seqno; - stored_seqno = stored_seqno + 1; + throw_unless(43, signature_auth_disabled); + signature_auth_disabled = false; } else { ;; disallow - throw_unless(43, stored_seqno >= 0); + throw_if(43, signature_auth_disabled); ds = ds.skip_bits(size::stored_subwallet + size::public_key); var extensions_is_not_null = ds.preload_uint(1); throw_unless(42, extensions_is_not_null); - ;; Corner case: 0 -> 1 -> -1 - ;; 123 -> 124 -> -124 - stored_seqno = stored_seqno + 1; - stored_seqno = - stored_seqno; + signature_auth_disabled = true; } set_data(begin_cell() - .store_int(stored_seqno, size::stored_seqno) + .store_int(signature_auth_disabled, size::signature_auth_disabled) + .store_uint(stored_seqno, size::stored_seqno) .store_slice(immutable_tail) ;; stored_subwallet ~ public_key ~ extensions .end_cell()); } @@ -153,7 +150,7 @@ cell verify_actions(cell c5, int is_external) inline { () process_signed_request(slice full_body, int is_external) impure inline { ifnot (is_external) { ;; Additional check to make sure that there are enough bits for reading (+1 for actual actions flag) - return_if(full_body.slice_bits() < 32 + size::subwallet_id + size::valid_until + size::msg_seqno + 1 + 512); + return_if(full_body.slice_bits() < 32 + size::signature_auth_disabled + size::subwallet_id + size::valid_until + size::msg_seqno + 1 + 512); } ;; The precise order of operations here is VERY important. Any other order results in unneccessary stack shuffles. @@ -164,7 +161,9 @@ cell verify_actions(cell c5, int is_external) inline { var (subwallet_id, valid_until, msg_seqno) = (cs~load_uint(size::subwallet_id), cs~load_uint(size::valid_until), cs~load_uint(size::msg_seqno)); var ds = get_data().begin_parse(); - var stored_seqno = ds~load_int(size::stored_seqno); + var signature_auth_disabled = ds~load_int(size::signature_auth_disabled); + + var stored_seqno = ds~load_uint(size::stored_seqno); var immutable_tail = ds; ;; stored_subwallet ~ public_key ~ extensions var stored_subwallet = ds~load_uint(size::stored_subwallet); var public_key = ds.preload_uint(size::public_key); @@ -180,15 +179,16 @@ cell verify_actions(cell c5, int is_external) inline { ;; TODO: Consider moving signed into separate ref, slice_hash consumes 500 gas just like cell creation! int signature_is_valid = check_signature(slice_hash(signed), signature, public_key); if (is_external) { + throw_if(32, signature_auth_disabled); throw_unless(35, signature_is_valid); } else { + if (signature_auth_disabled) { + return(); + } ifnot (signature_is_valid) { return(); } } - ;; If public key is disabled, stored_seqno is strictly less than zero: stored_seqno < 0 - ;; However, msg_seqno is uint, therefore it can be only greater or equal to zero: msg_seqno >= 0 - ;; Thus, if public key is disabled, these two domains NEVER intersect, and additional check is not needed throw_unless(33, msg_seqno == stored_seqno); throw_unless(34, subwallet_id == stored_subwallet); throw_if(36, valid_until <= now()); @@ -200,7 +200,8 @@ cell verify_actions(cell c5, int is_external) inline { ;; Store and commit the seqno increment to prevent replays even if the subsequent requests fail. stored_seqno = stored_seqno + 1; set_data(begin_cell() - .store_int(stored_seqno, size::stored_seqno) + .store_int(false, size::signature_auth_disabled) ;; it cannot be true, otherwise execution would not get here + .store_uint(stored_seqno, size::stored_seqno) .store_slice(immutable_tail) ;; stored_subwallet ~ public_key ~ extensions .end_cell()); @@ -252,7 +253,7 @@ cell verify_actions(cell c5, int is_external) inline { var ds = get_data().begin_parse(); ;; It is not required to read this data here, maybe ext is doing simple transfer where those are not needed - var extensions = ds.skip_bits(size::stored_seqno + size::stored_subwallet + size::public_key).preload_dict(); + var extensions = ds.skip_bits(size::signature_auth_disabled + size::stored_seqno + size::stored_subwallet + size::public_key).preload_dict(); ;; Note that some random contract may have deposited funds with this prefix, ;; so we accept the funds silently instead of throwing an error (wallet v4 does the same). @@ -278,25 +279,25 @@ cell verify_actions(cell c5, int is_external) inline { int seqno() method_id { ;; Use absolute value to do not confuse apps with negative seqno if key is disabled - return abs(get_data().begin_parse().preload_int(size::stored_seqno)); + return get_data().begin_parse().skip_bits(size::signature_auth_disabled).preload_uint(size::stored_seqno); } int get_wallet_id() method_id { - return get_data().begin_parse().skip_bits(size::stored_seqno).preload_uint(size::stored_subwallet); + return get_data().begin_parse().skip_bits(size::signature_auth_disabled + size::stored_seqno).preload_uint(size::stored_subwallet); } int get_public_key() method_id { - var cs = get_data().begin_parse().skip_bits(size::stored_seqno + size::stored_subwallet); + var cs = get_data().begin_parse().skip_bits(size::signature_auth_disabled + size::stored_seqno + size::stored_subwallet); return cs.preload_uint(size::public_key); } ;; Returns raw dictionary (or null if empty) where keys are packed addresses and the `wc` is stored in leafs. ;; User should unpack the address using the same packing function using `wc` to restore the original address. cell get_extensions() method_id { - var ds = get_data().begin_parse().skip_bits(size::stored_seqno + size::stored_subwallet + size::public_key); + var ds = get_data().begin_parse().skip_bits(size::signature_auth_disabled + size::stored_seqno + size::stored_subwallet + size::public_key); return ds~load_dict(); } int get_is_signature_auth_allowed() method_id { - return get_data().begin_parse().preload_int(size::stored_seqno) >= 0; + return ~ get_data().begin_parse().preload_int(size::signature_auth_disabled); } diff --git a/scripts/deployWalletV5.ts b/scripts/deployWalletV5.ts index 39f87e4..ce66fbb 100644 --- a/scripts/deployWalletV5.ts +++ b/scripts/deployWalletV5.ts @@ -15,6 +15,7 @@ export async function run(provider: NetworkProvider) { const walletV5 = provider.open( WalletV5.createFromConfig( { + signature_auth_disabled: false, seqno: 0, walletId: new WalletId({ networkGlobalId: -3 }).serialized, // testnet publicKey: keypair.publicKey, diff --git a/tests/wallet-v5-extensions.spec.ts b/tests/wallet-v5-extensions.spec.ts index ae91fdd..9b8522d 100644 --- a/tests/wallet-v5-extensions.spec.ts +++ b/tests/wallet-v5-extensions.spec.ts @@ -68,6 +68,7 @@ describe('Wallet V5 extensions auth', () => { walletV5 = blockchain.openContract( WalletV5.createFromConfig( { + signature_auth_disabled: false, seqno: 0, walletId: WALLET_ID.serialized, publicKey: keypair.publicKey, @@ -391,10 +392,7 @@ describe('Wallet V5 extensions auth', () => { expect(isSignatureAuthAllowed1).toEqual(-1); const contract_seqno = await walletV5.getSeqno(); - expect(contract_seqno).toEqual(seqno + 2); - - // Allowing or disallowing signature auth increments seqno, need to re-read - seqno = contract_seqno; + expect(contract_seqno).toEqual(seqno); const testReceiver = Address.parse('EQAvDfWFG0oYX19jwNDNBBL1rKNT9XfaGP9HyTb5nb2Eml6y'); const forwardValue = toNano(0.001); @@ -480,10 +478,7 @@ describe('Wallet V5 extensions auth', () => { expect(isSignatureAuthAllowed1).toEqual(-1); const contract_seqno = await walletV5.getSeqno(); - expect(contract_seqno).toEqual(seqno + 2); - - // Allowing or disallowing signature auth increments seqno, need to re-read - seqno = contract_seqno; + expect(contract_seqno).toEqual(seqno); const testReceiver = Address.parse('EQAvDfWFG0oYX19jwNDNBBL1rKNT9XfaGP9HyTb5nb2Eml6y'); const forwardValue = toNano(0.001); diff --git a/tests/wallet-v5-external.spec.ts b/tests/wallet-v5-external.spec.ts index 69e26c2..5c6ee97 100644 --- a/tests/wallet-v5-external.spec.ts +++ b/tests/wallet-v5-external.spec.ts @@ -58,6 +58,7 @@ describe('Wallet V5 sign auth external', () => { const _walletV5 = blockchain.openContract( WalletV5.createFromConfig( { + signature_auth_disabled: params?.signature_auth_disabled ?? false, seqno: params?.seqno ?? 0, walletId: params?.walletId ?? WALLET_ID.serialized, publicKey: params?.publicKey ?? _keypair.publicKey, @@ -100,6 +101,7 @@ describe('Wallet V5 sign auth external', () => { walletV5 = blockchain.openContract( WalletV5.createFromConfig( { + signature_auth_disabled: false, seqno: 0, walletId: WALLET_ID.serialized, publicKey: keypair.publicKey, @@ -746,7 +748,7 @@ describe('Wallet V5 sign auth external', () => { expect(isSignatureAuthAllowed).toEqual(0); const contract_seqno = await walletV5.getSeqno(); - expect(contract_seqno).toEqual(seqno + 1); + expect(contract_seqno).toEqual(seqno); }); it('Should add ext and disallow signature auth in separate txs', async () => { @@ -797,7 +799,7 @@ describe('Wallet V5 sign auth external', () => { expect(isSignatureAuthAllowed2).toEqual(0); const contract_seqno = await walletV5.getSeqno(); - expect(contract_seqno).toEqual(seqno + 1); + expect(contract_seqno).toEqual(seqno); }); it('Should add ext, disallow sign, allow sign, remove ext in one tx; send in other', async () => { @@ -823,10 +825,7 @@ describe('Wallet V5 sign auth external', () => { expect(isSignatureAuthAllowed).toEqual(-1); const contract_seqno = await walletV5.getSeqno(); - expect(contract_seqno).toEqual(seqno + 2); - - // Allowing or disallowing signature auth increments seqno, need to re-read - seqno = contract_seqno; + expect(contract_seqno).toEqual(seqno); const testReceiver = Address.parse('EQAvDfWFG0oYX19jwNDNBBL1rKNT9XfaGP9HyTb5nb2Eml6y'); const forwardValue = toNano(0.001); @@ -932,7 +931,7 @@ describe('Wallet V5 sign auth external', () => { expect(isSignatureAuthAllowed).toEqual(0); const contract_seqno = await walletV5.getSeqno(); - expect(contract_seqno).toEqual(seqno + 1); + expect(contract_seqno).toEqual(seqno); await disableConsoleError(() => expect(walletV5.sendExternalSignedMessage(createBody(packActionsList([])))).rejects.toThrow() diff --git a/tests/wallet-v5-get.spec.ts b/tests/wallet-v5-get.spec.ts index 6ec99d2..c6bca5e 100644 --- a/tests/wallet-v5-get.spec.ts +++ b/tests/wallet-v5-get.spec.ts @@ -31,6 +31,7 @@ describe('Wallet V5 get methods', () => { walletV5 = blockchain.openContract( WalletV5.createFromConfig( { + signature_auth_disabled: params?.signature_auth_disabled ?? false, seqno: params?.seqno ?? 0, walletId: params?.walletId ?? WALLET_ID.serialized, publicKey: params?.publicKey ?? keypair.publicKey, diff --git a/tests/wallet-v5-internal.spec.ts b/tests/wallet-v5-internal.spec.ts index 6459520..dc51b7c 100644 --- a/tests/wallet-v5-internal.spec.ts +++ b/tests/wallet-v5-internal.spec.ts @@ -51,6 +51,7 @@ describe('Wallet V5 sign auth internal', () => { const _walletV5 = blockchain.openContract( WalletV5.createFromConfig( { + signature_auth_disabled: params?.signature_auth_disabled ?? false, seqno: params?.seqno ?? 0, walletId: params?.walletId ?? WALLET_ID.serialized, publicKey: params?.publicKey ?? _keypair.publicKey, @@ -93,6 +94,7 @@ describe('Wallet V5 sign auth internal', () => { walletV5 = blockchain.openContract( WalletV5.createFromConfig( { + signature_auth_disabled: false, seqno: 0, walletId: WALLET_ID.serialized, publicKey: keypair.publicKey, @@ -941,7 +943,7 @@ describe('Wallet V5 sign auth internal', () => { expect(isSignatureAuthAllowed).toEqual(0); const contract_seqno = await walletV5.getSeqno(); - expect(contract_seqno).toEqual(seqno + 1); + expect(contract_seqno).toEqual(seqno); }); it('Should add ext and disallow signature auth in separate txs', async () => { @@ -1008,7 +1010,7 @@ describe('Wallet V5 sign auth internal', () => { expect(isSignatureAuthAllowed2).toEqual(0); const contract_seqno = await walletV5.getSeqno(); - expect(contract_seqno).toEqual(seqno + 1); + expect(contract_seqno).toEqual(seqno); }); it('Should add ext, disallow sign, allow sign, remove ext in one tx; send in other', async () => { @@ -1041,10 +1043,7 @@ describe('Wallet V5 sign auth internal', () => { expect(isSignatureAuthAllowed).toEqual(-1); const contract_seqno = await walletV5.getSeqno(); - expect(contract_seqno).toEqual(seqno + 2); - - // Allowing or disallowing signature auth increments seqno, need to re-read - seqno = contract_seqno; + expect(contract_seqno).toEqual(seqno); const testReceiver = Address.parse('EQAvDfWFG0oYX19jwNDNBBL1rKNT9XfaGP9HyTb5nb2Eml6y'); const forwardValue = toNano(0.001); @@ -1175,7 +1174,7 @@ describe('Wallet V5 sign auth internal', () => { expect(isSignatureAuthAllowed).toEqual(0); const contract_seqno = await walletV5.getSeqno(); - expect(contract_seqno).toEqual(seqno + 1); + expect(contract_seqno).toEqual(seqno); const testReceiver = Address.parse('EQAvDfWFG0oYX19jwNDNBBL1rKNT9XfaGP9HyTb5nb2Eml6y'); const forwardValue = toNano(0.001); @@ -1194,7 +1193,7 @@ describe('Wallet V5 sign auth internal', () => { (receipt2.transactions[1].description as TransactionDescriptionGeneric) .computePhase as TransactionComputeVm ).exitCode - ).toEqual(33); + ).toEqual(0); expect(receipt2.transactions).not.toHaveTransaction({ from: walletV5.address, diff --git a/types.tlb b/types.tlb index 0ac3940..6390684 100644 --- a/types.tlb +++ b/types.tlb @@ -27,4 +27,4 @@ actions$_ {m:#} {n:#} actions:(ActionList n m) = InnerRequest; // Contract state wallet_id$_ global_id:int32 wc:int8 version:(## 8) subwallet_number:(## 32) = WalletID; -contract_state$_ seqno:int33 wallet_id:WalletID public_key:(## 256) extensions_dict:(HashmapE 256 int8) = ContractState; +contract_state$_ signature_auth_disabled:(## 1) seqno:# wallet_id:WalletID public_key:(## 256) extensions_dict:(HashmapE 256 int8) = ContractState; diff --git a/wrappers/wallet-v5.ts b/wrappers/wallet-v5.ts index 89c991b..3d5fd45 100644 --- a/wrappers/wallet-v5.ts +++ b/wrappers/wallet-v5.ts @@ -15,6 +15,7 @@ import { import { bufferToBigInt } from '../tests/utils'; export type WalletV5Config = { + signature_auth_disabled: boolean; seqno: number; walletId: bigint; publicKey: Buffer; @@ -23,7 +24,8 @@ export type WalletV5Config = { export function walletV5ConfigToCell(config: WalletV5Config): Cell { return beginCell() - .storeInt(config.seqno, 33) + .storeBit(config.signature_auth_disabled) + .storeUint(config.seqno, 32) .storeUint(config.walletId, 80) .storeBuffer(config.publicKey, 32) .storeDict(config.extensions, Dictionary.Keys.BigUint(256), Dictionary.Values.BigInt(8)) From a210caabc58b737d4c2fbe6e3676020b5c88dda3 Mon Sep 17 00:00:00 2001 From: Skydev0h Date: Fri, 14 Jun 2024 19:38:39 +0300 Subject: [PATCH 4/4] Fix `getExtensionsArray()` method when address hash starts with `00` Co-authored-by: Dmytro Polunin --- tests/wallet-v5-get.spec.ts | 11 ++++++++--- wrappers/wallet-v5.ts | 2 +- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/tests/wallet-v5-get.spec.ts b/tests/wallet-v5-get.spec.ts index c6bca5e..cb36bea 100644 --- a/tests/wallet-v5-get.spec.ts +++ b/tests/wallet-v5-get.spec.ts @@ -127,8 +127,11 @@ describe('Wallet V5 get methods', () => { }); it('Get extensions array', async () => { - const plugin1 = Address.parse('EQAvDfWFG0oYX19jwNDNBBL1rKNT9XfaGP9HyTb5nb2Eml6y'); - const plugin2 = Address.parse('EQA2pT4d8T7TyRsjW2BpGpGYga-lMA4JjQb4D2tc1PXMX5Bf'); + const plugin1 = Address.parse( + '0:0000F5851B4A185F5F63C0D0CD0412F5ACA353F577DA18FF47C936F99DBD0000' + ); + const plugin2 = Address.parse('EQAvDfWFG0oYX19jwNDNBBL1rKNT9XfaGP9HyTb5nb2Eml6y'); + const plugin3 = Address.parse('EQA2pT4d8T7TyRsjW2BpGpGYga-lMA4JjQb4D2tc1PXMX5Bf'); const extensions: Dictionary = Dictionary.empty( Dictionary.Keys.BigUint(256), @@ -136,13 +139,15 @@ describe('Wallet V5 get methods', () => { ); extensions.set(packAddress(plugin1), BigInt(plugin1.workChain)); extensions.set(packAddress(plugin2), BigInt(plugin2.workChain)); + extensions.set(packAddress(plugin3), BigInt(plugin3.workChain)); await deploy({ extensions }); const actual = await walletV5.getExtensionsArray(); - expect(actual.length).toBe(2); + expect(actual.length).toBe(3); expect(actual[0].equals(plugin1)).toBeTruthy(); expect(actual[1].equals(plugin2)).toBeTruthy(); + expect(actual[2].equals(plugin3)).toBeTruthy(); }); it('Get empty extensions array', async () => { diff --git a/wrappers/wallet-v5.ts b/wrappers/wallet-v5.ts index 3d5fd45..b62870e 100644 --- a/wrappers/wallet-v5.ts +++ b/wrappers/wallet-v5.ts @@ -235,7 +235,7 @@ export class WalletV5 implements Contract { return dict.keys().map(key => { const wc = dict.get(key)!; const addressHex = key; - return Address.parseRaw(`${wc}:${addressHex.toString(16)}`); + return Address.parseRaw(`${wc}:${addressHex.toString(16).padStart(64, '0')}`); }); } }