Skip to content

Commit

Permalink
[Refactor] Get rid of control variables (#341)
Browse files Browse the repository at this point in the history
* Implement LockableFunction class

* Use LockableFunction in wallet.js

* Implement the lockablefunction with a function in place of a class

* Remove isCreatingTx

* Forgot to call the function

* fix bad merge

* Update JSdoc to take in account generic parameters

* fix bad merge

* add lockable function unit tests

* review fix
  • Loading branch information
panleone authored Apr 22, 2024
1 parent 48282b5 commit 640bdf9
Show file tree
Hide file tree
Showing 5 changed files with 142 additions and 102 deletions.
44 changes: 19 additions & 25 deletions scripts/composables/use_wallet.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { strCurrency } from '../settings.js';
import { cMarket } from '../settings.js';
import { ledgerSignTransaction } from '../ledger.js';
import { defineStore } from 'pinia';
import { lockableFunction } from '../lock.js';

/**
* This is the middle ground between vue and the wallet class
Expand All @@ -23,9 +24,6 @@ export const useWallet = defineStore('wallet', () => {
const isEncrypted = ref(true);
const loadFromDisk = () => wallet.loadFromDisk();
const hasShield = ref(wallet.hasShield());
// True only iff a shield transaction is being created
// Transparent txs are so fast that we don't need to keep track of them.
const isCreatingTx = ref(false);

const setMasterKey = async (mk) => {
wallet.setMasterKey(mk);
Expand Down Expand Up @@ -75,30 +73,27 @@ export const useWallet = defineStore('wallet', () => {
getEventEmitter().on('shield-loaded-from-disk', () => {
hasShield.value = wallet.hasShield();
});
const createAndSendTransaction = lockableFunction(
async (network, address, value, opts) => {
const tx = wallet.createTransaction(address, value, opts);
if (wallet.isHardwareWallet()) {
await ledgerSignTransaction(wallet, tx);
} else {
await wallet.sign(tx);
}
const res = await network.sendTransaction(tx.serialize());
if (res) {
await wallet.addTransaction(tx);
} else {
wallet.discardTransaction(tx);
}
return res;
}
);

getEventEmitter().on('toggle-network', async () => {
isEncrypted.value = await hasEncryptedWallet();
});
getEventEmitter().on(
'shield-transaction-creation-update',
async (_, finished) => {
isCreatingTx.value = !finished;
}
);
const createAndSendTransaction = async (network, address, value, opts) => {
const tx = wallet.createTransaction(address, value, opts);
if (wallet.isHardwareWallet()) {
await ledgerSignTransaction(wallet, tx);
} else {
await wallet.sign(tx);
}
const res = await network.sendTransaction(tx.serialize());
if (res) {
await wallet.addTransaction(tx);
} else {
wallet.discardTransaction(tx);
}
return res;
};

getEventEmitter().on('balance-update', async () => {
balance.value = wallet.balance;
Expand Down Expand Up @@ -132,7 +127,6 @@ export const useWallet = defineStore('wallet', () => {
hasShield,
shieldBalance,
pendingShieldBalance,
isCreatingTx,
immatureBalance,
currency,
price,
Expand Down
3 changes: 1 addition & 2 deletions scripts/dashboard/Dashboard.vue
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import ExportPrivKey from './ExportPrivKey.vue';
import RestoreWallet from './RestoreWallet.vue';
import {
createAlert,
isBase64,
isExchangeAddress,
isShieldAddress,
isValidPIVXAddress,
Expand Down Expand Up @@ -227,7 +226,7 @@ async function send(address, amount, useShieldInputs) {
}
// Make sure we are not already creating a (shield) tx
if (wallet.isCreatingTx) {
if (wallet.createAndSendTransaction.isLocked()) {
return createAlert(
'warning',
'Already creating a transaction! please wait for it to finish'
Expand Down
21 changes: 21 additions & 0 deletions scripts/lock.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/**
* Implement a lockable object
* @template T
* @param {T} f - the function on which we perform the lock
* @returns T & { isLocked: () => bool }
*/
export const lockableFunction = (f) => {
let lock = false;
const g = async (...args) => {
try {
if (!lock) {
lock = true;
return await f(...args);
}
} finally {
lock = false;
}
};
g.isLocked = () => lock;
return g;
};
144 changes: 69 additions & 75 deletions scripts/wallet.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { bytesToHex, hexToBytes, sleep, startBatch } from './utils.js';
import { strHardwareName } from './ledger.js';
import { OutpointState, Mempool } from './mempool.js';
import { getEventEmitter } from './event_bus.js';
import { lockableFunction } from './lock.js';

import {
isP2CS,
Expand Down Expand Up @@ -90,7 +91,6 @@ export class Wallet {
#mempool;

#isSynced = false;
#isFetchingLatestBlocks = false;
/**
* The height of the last processed block in the wallet
* @type {number}
Expand Down Expand Up @@ -166,7 +166,7 @@ export class Wallet {
}

get isSyncing() {
return this.#syncing;
return this.sync.isLocked();
}

wipePrivateData() {
Expand Down Expand Up @@ -687,31 +687,24 @@ export class Wallet {
}
return histTXs;
}
#syncing = false;

async sync() {
if (this.#isSynced || this.#syncing) {
sync = lockableFunction(async () => {
if (this.#isSynced) {
throw new Error('Attempting to sync when already synced');
}
try {
this.#syncing = true;
await this.loadFromDisk();
await this.loadShieldFromDisk();
// Let's set the last processed block 5 blocks behind the actual chain tip
// This is just to be sure since blockbook (as we know)
// usually does not return txs of the actual last block.
this.#lastProcessedBlock = blockCount - 5;
await this.#transparentSync();
if (this.hasShield()) {
await this.#syncShield();
}
this.#isSynced = true;
} finally {
this.#syncing = false;
await this.loadFromDisk();
await this.loadShieldFromDisk();
// Let's set the last processed block 5 blocks behind the actual chain tip
// This is just to be sure since blockbook (as we know)
// usually does not return txs of the actual last block.
this.#lastProcessedBlock = blockCount - 5;
await this.#transparentSync();
if (this.hasShield()) {
await this.#syncShield();
}
this.#isSynced = true;
// Update both activities post sync
getEventEmitter().emit('new-tx');
}
});

async #transparentSync() {
if (!this.isLoaded() || this.#isSynced) return;
Expand Down Expand Up @@ -820,65 +813,66 @@ export class Wallet {
}
});
}
/**
* Update the shield object with the latest blocks
* @param{number} blockCount - block count
*/
async getLatestBlocks(blockCount) {
// Exit if this function is still processing
// (this might take some time if we had many consecutive blocks without shield txs)
if (this.#isFetchingLatestBlocks) return;
this.#isFetchingLatestBlocks = true;

const cNet = getNetwork();
let block;
// Don't ask for the exact last block that arrived,
// since it takes around 1 minute for blockbook to make it API available
for (
let blockHeight = this.#lastProcessedBlock + 1;
blockHeight < blockCount;
blockHeight++
) {
try {
const block = await cNet.getBlock(blockHeight, false);
if (block.txs) {
if (this.hasShield()) {
if (blockHeight > this.#shield.getLastSyncedBlock()) {
await this.#shield.handleBlock(block);
}
}
for (const tx of block.txs) {
const parsed = Transaction.fromHex(tx.hex);
parsed.blockHeight = blockHeight;
parsed.blockTime = tx.blocktime;
// Avoid wasting memory on txs that do not regard our wallet
if (this.#mempool.ownTransaction(parsed)) {
await wallet.addTransaction(parsed);
getLatestBlocks = lockableFunction(
/**
* Update the shield object with the latest blocks
* @param{number} blockCount - block count
*/
async (blockCount) => {
// Exit if there is no shield loaded
if (!this.hasShield()) return;
const cNet = getNetwork();
let block;
// Don't ask for the exact last block that arrived,
// since it takes around 1 minute for blockbook to make it API available
for (
let blockHeight = this.#lastProcessedBlock + 1;
blockHeight < blockCount;
blockHeight++
) {
try {
block = await cNet.getBlock(blockHeight);
if (block.txs) {
if (this.hasShield()) {
if (
blockHeight > this.#shield.getLastSyncedBlock()
) {
await this.#shield.handleBlock(block);
}
for (const tx of block.txs) {
const parsed = Transaction.fromHex(tx.hex);
parsed.blockHeight = blockHeight;
parsed.blockTime = tx.blocktime;
// Avoid wasting memory on txs that do not regard our wallet
if (this.#mempool.ownTransaction(parsed)) {
await wallet.addTransaction(parsed);
}
}
}
} else {
break;
}
} else {
this.#lastProcessedBlock = blockHeight;
} catch (e) {
console.error(e);
break;
}
this.#lastProcessedBlock = blockHeight;
} catch (e) {
console.error(e);
break;
}
}
this.#isFetchingLatestBlocks = false;

// SHIELD-only checks
if (this.hasShield()) {
if (block?.finalSaplingRoot)
if (
!(await this.#checkShieldSaplingRoot(
block.finalsaplingroot
))
)
return;
await this.saveShieldOnDisk();

// SHIELD-only checks
if (this.hasShield()) {
if (block?.finalSaplingRoot)
if (
!(await this.#checkShieldSaplingRoot(
block.finalsaplingroot
))
)
return;
await this.saveShieldOnDisk();
}
}
}
);

async #checkShieldSaplingRoot(networkSaplingRoot) {
const saplingRoot = bytesToHex(
Expand All @@ -889,7 +883,7 @@ export class Wallet {
createAlert('warning', translation.badSaplingRoot, 5000);
this.#mempool = new Mempool();
// TODO: take the wallet creation height in input from users
this.#shield.reloadFromCheckpoint(4200000);
await this.#shield.reloadFromCheckpoint(4200000);
await this.#transparentSync();
await this.#syncShield();
return false;
Expand Down
32 changes: 32 additions & 0 deletions tests/unit/lockable_function.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { lockableFunction } from '../../scripts/lock.js';
import { sleep } from '../../scripts/utils.js';
import { beforeEach, it } from 'vitest';

describe('Lockable function tests', () => {
let test_function;
const sleep_time = 1000;
beforeEach(() => {
test_function = lockableFunction(async (str_input) => {
await sleep(sleep_time);
return str_input;
});
});
it('Lockable function returns the correct value', async () => {
expect(await test_function('test_locks')).toBe('test_locks');
});
it('Lockable function gives the correct value for the lock', async () => {
// At the beginning there is no lock
expect(test_function.isLocked()).toBeFalsy();
test_function('test_locks');
await sleep(sleep_time / 2);
// When the function is running the lock is acquired
expect(test_function.isLocked()).toBeTruthy();
await sleep(sleep_time / 2);
// When the function stop running the lock is dropped
expect(test_function.isLocked()).toBeFalsy();
});
it("Calling when locked doesn't make the function run twice", async () => {
test_function('test_locks');
expect(await test_function('test_locks')).toBeUndefined();
});
});

0 comments on commit 640bdf9

Please sign in to comment.