Skip to content

Commit

Permalink
Moved signature_auth_disable to separate variable from seq_no sign (3)
Browse files Browse the repository at this point in the history
  • Loading branch information
Skydev0h committed Jun 14, 2024
1 parent 7d1d144 commit 1f9b0a3
Show file tree
Hide file tree
Showing 9 changed files with 52 additions and 54 deletions.
2 changes: 1 addition & 1 deletion Specification.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
57 changes: 29 additions & 28 deletions contracts/wallet_v5.fc
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand All @@ -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()
Expand All @@ -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());
}
Expand Down Expand Up @@ -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.
Expand All @@ -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);
Expand All @@ -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());
Expand All @@ -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());

Expand Down Expand Up @@ -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).
Expand All @@ -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);
}
1 change: 1 addition & 0 deletions scripts/deployWalletV5.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
11 changes: 3 additions & 8 deletions tests/wallet-v5-extensions.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
13 changes: 6 additions & 7 deletions tests/wallet-v5-external.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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 () => {
Expand All @@ -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);
Expand Down Expand Up @@ -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()
Expand Down
1 change: 1 addition & 0 deletions tests/wallet-v5-get.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
15 changes: 7 additions & 8 deletions tests/wallet-v5-internal.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion types.tlb
Original file line number Diff line number Diff line change
Expand Up @@ -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;
4 changes: 3 additions & 1 deletion wrappers/wallet-v5.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
import { bufferToBigInt } from '../tests/utils';

export type WalletV5Config = {
signature_auth_disabled: boolean;
seqno: number;
walletId: bigint;
publicKey: Buffer;
Expand All @@ -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))
Expand Down

0 comments on commit 1f9b0a3

Please sign in to comment.