Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability for MPW to run explorer-less #427

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open
26 changes: 14 additions & 12 deletions scripts/dashboard/WalletBalance.vue
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<script setup>
import { cChainParams, COIN } from '../chain_params.js';
import { translation } from '../i18n';
import { translation, tr } from '../i18n';
import { ref, computed, toRefs } from 'vue';
import { beautifyNumber } from '../misc';
import { getEventEmitter } from '../event_bus';
Expand Down Expand Up @@ -130,11 +130,13 @@ const emit = defineEmits([
getEventEmitter().on(
'transparent-sync-status-update',
(i, totalPages, finished) => {
const str = tr(translation.syncStatusHistoryProgress, [
{ current: totalPages - i + 1 },
{ total: totalPages },
]);
(i, totalPages, finished, message) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
(i, totalPages, finished, message) => {
(i, totalPages, finished, error) => {

Can you call this error/warning?

const str =
message ||
tr(translation.syncStatusHistoryProgress, [
{ current: totalPages - i + 1 },
{ total: totalPages },
]);
const progress = ((totalPages - i) / totalPages) * 100;
syncTStr.value = str;
transparentProgressSyncing.value = progress;
Expand Down Expand Up @@ -510,16 +512,16 @@ function restoreWallet() {
</div>
<div style="width: 100%">
{{
transparentSyncing
? syncTStr
: `Syncing ${shieldBlockRemainingSyncing} Blocks...`
shieldSyncing
? `Syncing ${shieldBlockRemainingSyncing} Blocks...`
: syncTStr
}}
<LoadingBar
:show="true"
:percentage="
transparentSyncing
? transparentProgressSyncing
: shieldPercentageSyncing
shieldSyncing
? shieldPercentageSyncing
: transparentProgressSyncing
"
style="
border: 1px solid #932ecd;
Expand Down
104 changes: 56 additions & 48 deletions scripts/network.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,58 +92,47 @@ export class ExplorerNetwork extends Network {
}

/**
* Fetch a block from the explorer given the height
* Fetch a block from the current node given the height
* @param {number} blockHeight
* @param {boolean} skipCoinstake - if true coinstake tx will be skipped
* @returns {Promise<Object>} the block fetched from explorer
* @returns {Promise<Object>} the block
*/
async getBlock(blockHeight, skipCoinstake = false) {
const block = await this.safeFetchFromExplorer(
`/api/v2/block/${blockHeight}`
);
const newTxs = [];
// This is bad. We're making so many requests
// This is a quick fix to try to be compliant with the blockbook
// API, and not the PIVX extension.
// In the Blockbook API /block doesn't have any chain specific information
// Like hex, shield info or what not.
// We could change /getshieldblocks to /getshieldtxs?
// In addition, always skip the coinbase transaction and in case the coinstake one
// TODO: once v6.0 and shield stake is activated we might need to change this optimization
for (const tx of block.txs.slice(skipCoinstake ? 2 : 1)) {
const r = await fetch(
`${this.strUrl}/api/v2/tx-specific/${tx.txid}`
);
if (!r.ok) throw new Error('failed');
const newTx = await r.json();
newTxs.push(newTx);
}
block.txs = newTxs;
return block;
async getBlock(blockHeight) {
// First we fetch the blockhash (and strip RPC's quotes)
const strHash = (
await this.callRPC(`/getblockhash?params=${blockHeight}`, true)
).replace(/"/g, '');
// Craft a filter to retrieve only raw Tx hex and txid, also change "tx" to "txs"
const strFilter =
'&filter=' +
encodeURI(`. | .txs = [.tx[] | { hex: .hex, txid: .txid}]`);
// Fetch the full block (verbose)
return await this.callRPC(`/getblock?params=${strHash},2${strFilter}`);
}

/**
* Fetch the block height of the current explorer
* Fetch the block height of the current node
* @returns {Promise<number>} - Block height
*/
async getBlockCount() {
const { backend } = await (
await retryWrapper(fetchBlockbook, true, `/api/v2/api`)
).json();

return backend.blocks;
return parseInt(await this.callRPC('/getblockcount', true));
}

/**
* Fetch the latest block hash of the current explorer
* Fetch the latest block hash of the current explorer or fallback node
* @returns {Promise<string>} - Block hash
*/
async getBestBlockHash() {
const { backend } = await (
await retryWrapper(fetchBlockbook, true, `/api/v2/api`)
).json();
try {
// Attempt via Explorer first
const { backend } = await (
await retryWrapper(fetchBlockbook, true, `/api/v2/api`)
).json();

return backend.bestBlockHash;
return backend.bestBlockHash;
} catch {
// Use Nodes as a fallback
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like using nodes as a fallback for light RPC calls like getBestBlockHash , sendTransaction and getBlockCount: the code is more ugly and imo we should just make sure to have working explorers

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Imo we would need at least 2x (4) explorers to consider it a stable network, or we start relying more on Nodes instead, it is the only compromise to get MPW more stable.

return await this.callRPC('/getbestblockhash', true);
}
}

/**
Expand Down Expand Up @@ -272,19 +261,38 @@ export class ExplorerNetwork extends Network {

async sendTransaction(hex) {
try {
const data = await (
await retryWrapper(fetchBlockbook, true, '/api/v2/sendtx/', {
method: 'post',
body: hex,
})
).json();
// Attempt via Explorer first
let strTXID = '';
try {
const cData = await (
await retryWrapper(
fetchBlockbook,
true,
'/api/v2/sendtx/',
{
method: 'post',
body: hex,
}
)
).json();
// If there's no TXID, we throw any potential Blockbook errors
if (!cData.result || cData.result.length !== 64) throw cData;
strTXID = cData.result;
} catch {
// Use Nodes as a fallback
strTXID = await this.callRPC(
'/sendrawtransaction?params=' + hex,
true
);
strTXID = strTXID.replace(/"/g, '');
}

// Throw and catch if the data is not a TXID
if (!data.result || data.result.length !== 64) throw data;
// Throw and catch if there's no TXID
if (!strTXID || strTXID.length !== 64) throw strTXID;

debugLog(DebugTopics.NET, 'Transaction sent! ' + data.result);
getEventEmitter().emit('transaction-sent', true, data.result);
return data.result;
debugLog(DebugTopics.NET, 'Transaction sent! ' + strTXID);
getEventEmitter().emit('transaction-sent', true, strTXID);
return strTXID;
} catch (e) {
getEventEmitter().emit('transaction-sent', false, e);
return false;
Expand Down
47 changes: 34 additions & 13 deletions scripts/wallet.js
Original file line number Diff line number Diff line change
Expand Up @@ -693,17 +693,36 @@ export class Wallet {
if (this.#isSynced) {
throw new Error('Attempting to sync when already synced');
}
// While syncing the wallet ( DB read + network sync) disable the event balance-update
// While syncing the wallet (DB read + network sync) disable the event balance-update
// This is done to avoid a huge spam of event.
getEventEmitter().disableEvent('balance-update');

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();
this.#lastProcessedBlock = blockCount;
// Transparent sync is inherently less stable than Shield since it requires heavy
// explorer indexing, so we'll attempt once asynchronously, and if it fails, set a
// recurring "background" sync interval until it's finally successful.
try {
await this.#transparentSync();
} catch {
// We'll set a 5s interval sync until it's finally successful, then nuke the 'thread'.
const cThread = new AsyncInterval(async () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You cannot use AsyncInterval: the function sync() has to be blocked until transparentSync() is succesful, otherwise we end up in a state where the wallet is marked as synced, this.#isSynced = true, even if it isn't.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue with blocking it is Shield then becomes unusable unless Transparent syncs (right now, Shield can be used even if Transparent is unable).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally we would split the state so that we have transparentSynced and shieldSynced, so they are operable independently.

try {
await this.#transparentSync(true);
cThread.clearInterval();
} catch {
// Emit a transparent sync warning
getEventEmitter().emit(
'transparent-sync-status-update',
0,
0,
false,
'Explorers are unreachable, your wallet may not be fully synced!'
);
}
}, 5000);
}
if (this.hasShield()) {
await this.#syncShield();
}
Expand All @@ -714,8 +733,12 @@ export class Wallet {
getEventEmitter().emit('new-tx');
});

async #transparentSync() {
if (!this.isLoaded() || this.#isSynced) return;
/**
* Synchronise UTXOs via xpub/address from the current explorer.
* @param {boolean} [force=false] - Force transparent sync, regardless of state
*/
async #transparentSync(force = false) {
if ((!this.isLoaded() || this.#isSynced) && !force) return;
Comment on lines +740 to +741
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move the loadedCheck and synced in the sync function, so we can remove the force param.

Suggested change
async #transparentSync(force = false) {
if ((!this.isLoaded() || this.#isSynced) && !force) return;
async #transparentSync() {

It's only going to be called by sync anyways

const cNet = getNetwork();
const addr = this.getKeyToExport();
let nStartHeight = Math.max(
Expand Down Expand Up @@ -763,7 +786,7 @@ export class Wallet {
await startBatch(
async (i) => {
let block;
block = await cNet.getBlock(blockHeights[i], true);
block = await cNet.getBlock(blockHeights[i]);
downloaded++;
blocks[i] = block;
// We need to process blocks monotically
Expand Down Expand Up @@ -852,11 +875,9 @@ export class Wallet {
async (blockCount) => {
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;
let blockHeight = this.#lastProcessedBlock;
blockHeight <= blockCount;
blockHeight++
) {
try {
Expand Down
19 changes: 9 additions & 10 deletions tests/integration/wallet/sync.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ vi.mock('../../../scripts/network.js');
* @param{number} value - amounts to transfer
* @returns {Promise<void>}
*/
async function crateAndSendTransaction(wallet, address, value) {
async function createAndSendTransaction(wallet, address, value) {
const tx = wallet.createTransaction(address, value);
await wallet.sign(tx);
expect(getNetwork().sendTransaction(tx.serialize())).toBeTruthy();
Expand Down Expand Up @@ -65,23 +65,19 @@ describe('Wallet sync tests', () => {
it('Basic 2 wallets sync test', async () => {
// --- Verify that funds are received after sending a transaction ---
// The legacy wallet sends the HD wallet 0.05 PIVs
await crateAndSendTransaction(
await createAndSendTransaction(
walletLegacy,
walletHD.getCurrentAddress(),
0.05 * 10 ** 8
);

// Mint the block with the transaction
await mineAndSync();
// getLatestBlocks sync up until chain tip - 1 block,
// so at this point walletHD doesn't still know about the UTXO he received
expect(walletHD.balance).toBe(1 * 10 ** 8);
// mine an empty block and verify that the tx arrived
await mineAndSync();
expect(walletHD.balance).toBe((1 + 0.05) * 10 ** 8);

// Sends funds back to the legacy wallet and verify that he also correctly receives funds
const legacyBalance = walletLegacy.balance;
await crateAndSendTransaction(
await createAndSendTransaction(
walletHD,
walletLegacy.getCurrentAddress(),
1 * 10 ** 8
Expand All @@ -103,13 +99,16 @@ describe('Wallet sync tests', () => {
let newAddress = walletHD.getAddressFromPath(
path.slice(0, -1) + String(nAddress)
);
await crateAndSendTransaction(
// Create a Tx to the new account address
await createAndSendTransaction(
walletLegacy,
newAddress,
0.01 * 10 ** 8
);
await mineAndSync();
// Validate the balance of the HD wallet pre-tx-confirm
expect(walletHD.balance).toBe((1 + 0.01 * i) * 10 ** 8);
// Mine a block with the Tx
await mineAndSync();
}
});
it('recognizes immature balance', async () => {
Expand Down
Loading