Merge pull request #644 from keerifox/account-validation-fixes

Account/frontier validation fixes, improved UX upon encountering transaction errors

package-lock.json

@@ -50,7 +50,7 @@
         "hw-app-nano": "^1.3.0",
         "nano-base32": "^1.0.1",
         "nanocurrency": "^2.5.0",
-        "nanocurrency-web": "^1.2.1",
+        "nanocurrency-web": "^1.4.3",
         "ngx-clipboard": "^12.3.0",
         "node-hid": "^1.3.0",
         "qrcode": "^1.4.4",
@@ -6951,9 +6951,9 @@
       }
     },
     "node_modules/blakejs": {
-      "version": "1.1.0",
+      "version": "1.2.1",
-      "resolved": "https://registry.npmjs.org/blakejs/-/blakejs-1.1.0.tgz",
+      "resolved": "https://registry.npmjs.org/blakejs/-/blakejs-1.2.1.tgz",
-      "integrity": "sha1-ad+S75U6qIylGjLfarHFShVfx6U="
+      "integrity": "sha512-QXUSXI3QVc/gJME0dBpXrag1kbzOqCjCX8/b54ntNyW6sjtoqxqRk3LTmXzaJoh71zMsDCjM+47jS7XiwN/+fQ=="
     },
     "node_modules/blocking-proxy": {
       "version": "1.0.1",
@@ -7553,6 +7553,11 @@
       "integrity": "sha1-y5T662HIaWRR2zZTThQi+U8K7og=",
       "dev": true
     },
+    "node_modules/byte-base64": {
+      "version": "1.1.0",
+      "resolved": "https://registry.npmjs.org/byte-base64/-/byte-base64-1.1.0.tgz",
+      "integrity": "sha512-56cXelkJrVMdCY9V/3RfDxTh4VfMFCQ5km7B7GkIGfo4bcPL9aACyJLB0Ms3Ezu5rsHmLB2suis96z4fLM03DA=="
+    },
     "node_modules/bytes": {
       "version": "3.0.0",
       "resolved": "https://registry.npmjs.org/bytes/-/bytes-3.0.0.tgz",
@@ -14617,19 +14622,20 @@
       }
     },
     "node_modules/nanocurrency-web": {
-      "version": "1.2.1",
+      "version": "1.4.3",
-      "resolved": "https://registry.npmjs.org/nanocurrency-web/-/nanocurrency-web-1.2.1.tgz",
+      "resolved": "https://registry.npmjs.org/nanocurrency-web/-/nanocurrency-web-1.4.3.tgz",
-      "integrity": "sha512-OmGazZ4nl0PiUzL2Ag355cikU4yBgOa3ggEUsxF0T1dNqaxhoS7qeF7tEhDxwo7hqtqV49XwtQyWc7r000/6Dg==",
+      "integrity": "sha512-okmnHweUjZq3j/f2W5qYl4Ir4GAhGyL2BJJQEeZvo+qIHkdI4d7RbJDQKNK1VXAmdFZ4mElOPLGUBv6i+x+XRg==",
       "dependencies": {
-        "bignumber.js": "9.0.0",
+        "bignumber.js": "9.0.2",
-        "blakejs": "1.1.0",
+        "blakejs": "1.2.1",
+        "byte-base64": "1.1.0",
         "crypto-js": "3.1.9-1"
       }
     },
     "node_modules/nanocurrency-web/node_modules/bignumber.js": {
-      "version": "9.0.0",
+      "version": "9.0.2",
-      "resolved": "https://registry.npmjs.org/bignumber.js/-/bignumber.js-9.0.0.tgz",
+      "resolved": "https://registry.npmjs.org/bignumber.js/-/bignumber.js-9.0.2.tgz",
-      "integrity": "sha512-t/OYhhJ2SD+YGBQcjY8GzzDHEk9f3nerxjtfa6tlMXfe7frs/WozhvCNoGvpM0P3bNf3Gq5ZRMlGr5f3r4/N8A==",
+      "integrity": "sha512-GAcQvbpsM0pUb0zw1EI0KhQEZ+lRwR5fYaAp3vPOYuP7aDvGy6cVN6XHLauvF8SOga2y0dcLcjt3iQDTSEliyw==",
       "engines": {
         "node": "*"
       }
@@ -25728,9 +25734,9 @@
       }
     },
     "blakejs": {
-      "version": "1.1.0",
+      "version": "1.2.1",
-      "resolved": "https://registry.npmjs.org/blakejs/-/blakejs-1.1.0.tgz",
+      "resolved": "https://registry.npmjs.org/blakejs/-/blakejs-1.2.1.tgz",
-      "integrity": "sha1-ad+S75U6qIylGjLfarHFShVfx6U="
+      "integrity": "sha512-QXUSXI3QVc/gJME0dBpXrag1kbzOqCjCX8/b54ntNyW6sjtoqxqRk3LTmXzaJoh71zMsDCjM+47jS7XiwN/+fQ=="
     },
     "blocking-proxy": {
       "version": "1.0.1",
@@ -26193,6 +26199,11 @@
       "integrity": "sha1-y5T662HIaWRR2zZTThQi+U8K7og=",
       "dev": true
     },
+    "byte-base64": {
+      "version": "1.1.0",
+      "resolved": "https://registry.npmjs.org/byte-base64/-/byte-base64-1.1.0.tgz",
+      "integrity": "sha512-56cXelkJrVMdCY9V/3RfDxTh4VfMFCQ5km7B7GkIGfo4bcPL9aACyJLB0Ms3Ezu5rsHmLB2suis96z4fLM03DA=="
+    },
     "bytes": {
       "version": "3.0.0",
       "resolved": "https://registry.npmjs.org/bytes/-/bytes-3.0.0.tgz",
@@ -31507,19 +31518,20 @@
       }
     },
     "nanocurrency-web": {
-      "version": "1.2.1",
+      "version": "1.4.3",
-      "resolved": "https://registry.npmjs.org/nanocurrency-web/-/nanocurrency-web-1.2.1.tgz",
+      "resolved": "https://registry.npmjs.org/nanocurrency-web/-/nanocurrency-web-1.4.3.tgz",
-      "integrity": "sha512-OmGazZ4nl0PiUzL2Ag355cikU4yBgOa3ggEUsxF0T1dNqaxhoS7qeF7tEhDxwo7hqtqV49XwtQyWc7r000/6Dg==",
+      "integrity": "sha512-okmnHweUjZq3j/f2W5qYl4Ir4GAhGyL2BJJQEeZvo+qIHkdI4d7RbJDQKNK1VXAmdFZ4mElOPLGUBv6i+x+XRg==",
       "requires": {
-        "bignumber.js": "9.0.0",
+        "bignumber.js": "9.0.2",
-        "blakejs": "1.1.0",
+        "blakejs": "1.2.1",
+        "byte-base64": "1.1.0",
         "crypto-js": "3.1.9-1"
       },
       "dependencies": {
         "bignumber.js": {
-          "version": "9.0.0",
+          "version": "9.0.2",
-          "resolved": "https://registry.npmjs.org/bignumber.js/-/bignumber.js-9.0.0.tgz",
+          "resolved": "https://registry.npmjs.org/bignumber.js/-/bignumber.js-9.0.2.tgz",
-          "integrity": "sha512-t/OYhhJ2SD+YGBQcjY8GzzDHEk9f3nerxjtfa6tlMXfe7frs/WozhvCNoGvpM0P3bNf3Gq5ZRMlGr5f3r4/N8A=="
+          "integrity": "sha512-GAcQvbpsM0pUb0zw1EI0KhQEZ+lRwR5fYaAp3vPOYuP7aDvGy6cVN6XHLauvF8SOga2y0dcLcjt3iQDTSEliyw=="
         },
         "crypto-js": {
           "version": "3.1.9-1",

package.json

@@ -67,7 +67,7 @@
     "hw-app-nano": "^1.3.0",
     "nano-base32": "^1.0.1",
     "nanocurrency": "^2.5.0",
-    "nanocurrency-web": "^1.2.1",
+    "nanocurrency-web": "^1.4.3",
     "ngx-clipboard": "^12.3.0",
     "node-hid": "^1.3.0",
     "qrcode": "^1.4.4",

src/app/components/account-details/account-details.component.ts

@@ -971,10 +971,18 @@ export class AccountDetailsComponent implements OnInit, OnDestroy {
 
     receivableBlock.loading = true;
 
-    const createdReceiveBlockHash =
+    let createdReceiveBlockHash = null;
-      await this.nanoBlock.generateReceive(this.walletAccount, sourceBlock, this.wallet.isLedgerWallet());
+    let hasShownErrorNotification = false;
 
-    if (createdReceiveBlockHash) {
+    try {
+      createdReceiveBlockHash =
+        await this.nanoBlock.generateReceive(this.walletAccount, sourceBlock, this.wallet.isLedgerWallet());
+    } catch (err) {
+      this.notifications.sendError('Error receiving transaction: ' + err.message);
+      hasShownErrorNotification = true;
+    }
+
+    if (createdReceiveBlockHash != null) {
       receivableBlock.received = true;
       this.mobileTransactionMenuModal.hide();
       this.notifications.removeNotification('success-receive');
@@ -982,8 +990,10 @@ export class AccountDetailsComponent implements OnInit, OnDestroy {
       // clear the list of pending blocks. Updated again with reloadBalances()
       this.wallet.clearPendingBlocks();
     } else {
+      if (hasShownErrorNotification === false) {
         if (!this.wallet.isLedgerWallet()) {
-        this.notifications.sendError(`There was a problem receiving the transaction, try manually!`, {length: 10000});
+          this.notifications.sendError(`Error receiving transaction, please try again`, {length: 10000});
+        }
       }
     }
 

src/app/components/receive/receive.component.ts

@@ -357,10 +357,18 @@ export class ReceiveComponent implements OnInit, OnDestroy {
     }
     receivableBlock.loading = true;
 
-    const createdReceiveBlockHash =
+    let createdReceiveBlockHash = null;
-      await this.nanoBlock.generateReceive(walletAccount, sourceBlock, this.walletService.isLedgerWallet());
+    let hasShownErrorNotification = false;
 
-    if (createdReceiveBlockHash) {
+    try {
+      createdReceiveBlockHash =
+        await this.nanoBlock.generateReceive(walletAccount, sourceBlock, this.walletService.isLedgerWallet());
+    } catch (err) {
+      this.notificationService.sendError('Error receiving transaction: ' + err.message);
+      hasShownErrorNotification = true;
+    }
+
+    if (createdReceiveBlockHash != null) {
       receivableBlock.received = true;
       this.mobileTransactionMenuModal.hide();
       this.notificationService.removeNotification('success-receive');
@@ -369,8 +377,10 @@ export class ReceiveComponent implements OnInit, OnDestroy {
       // list also updated with reloadBalances but not if called too fast
       this.walletService.removePendingBlock(receivableBlock.hash);
     } else {
+      if (hasShownErrorNotification === false) {
         if (!this.walletService.isLedgerWallet()) {
-        this.notificationService.sendError(`There was a problem receiving the transaction, try manually!`, {length: 10000});
+          this.notificationService.sendError(`Error receiving transaction, please try again`, {length: 10000});
+        }
       }
     }
 

src/app/components/representatives/representatives.component.ts

@@ -341,7 +341,7 @@ export class RepresentativesComponent implements OnInit {
           this.notifications.sendError(`Error changing representative for ${account.id}, please try again`);
         }
       } catch (err) {
-        this.notifications.sendError(err.message);
+        this.notifications.sendError('Error changing representative: ' + err.message);
       }
     }
 

src/app/services/nano-block.service.ts

@@ -8,6 +8,7 @@ import {AppSettingsService} from './app-settings.service';
 import {LedgerService} from './ledger.service';
 import { WalletAccount } from './wallet.service';
 import {BehaviorSubject} from 'rxjs';
+import { tools as nanocurrencyWebTools } from 'nanocurrency-web';
 const nacl = window['nacl'];
 
 @Injectable()
@@ -42,6 +43,10 @@ export class NanoBlockService {
     const toAcct = await this.api.accountInfo(walletAccount.id);
     if (!toAcct) throw new Error(`Account must have an open block first`);
 
+    const walletAccountPublicKey = this.util.account.getAccountPublicKey(walletAccount.id);
+
+    await this.validateAccount(toAcct, walletAccountPublicKey);
+
     const balance = new BigNumber(toAcct.balance);
     const balanceDecimal = balance.toString(10);
     const link = this.zeroHash;
@@ -74,7 +79,6 @@ export class NanoBlockService {
         return;
       }
     } else {
-      this.validateAccount(toAcct);
       this.signStateBlock(walletAccount, blockData);
     }
 
@@ -188,6 +192,10 @@ export class NanoBlockService {
     const fromAccount = await this.api.accountInfo(walletAccount.id);
     if (!fromAccount) throw new Error(`Unable to get account information for ${walletAccount.id}`);
 
+    const walletAccountPublicKey = this.util.account.getAccountPublicKey(walletAccount.id);
+
+    await this.validateAccount(fromAccount, walletAccountPublicKey);
+
     const remaining = new BigNumber(fromAccount.balance).minus(rawAmount);
     const remainingDecimal = remaining.toString(10);
 
@@ -222,7 +230,6 @@ export class NanoBlockService {
         return;
       }
     } else {
-      this.validateAccount(fromAccount);
       this.signStateBlock(walletAccount, blockData);
     }
 
@@ -245,6 +252,10 @@ export class NanoBlockService {
 
   async generateReceive(walletAccount, sourceBlock, ledger = false) {
     const toAcct = await this.api.accountInfo(walletAccount.id);
+    const walletAccountPublicKey = this.util.account.getAccountPublicKey(walletAccount.id);
+
+    await this.validateAccount(toAcct, walletAccountPublicKey);
+
     let workBlock = null;
 
     const openEquiv = !toAcct || !toAcct.frontier;
@@ -294,11 +305,10 @@ export class NanoBlockService {
         return;
       }
     } else {
-      this.validateAccount(toAcct);
       this.signStateBlock(walletAccount, blockData);
     }
 
-    workBlock = openEquiv ? this.util.account.getAccountPublicKey(walletAccount.id) : previousBlock;
+    workBlock = openEquiv ? walletAccountPublicKey : previousBlock;
     if (!this.workPool.workExists(workBlock)) {
       this.notifications.sendInfo(`Generating Proof of Work...`, { identifier: 'pow', length: 0 });
     }
@@ -391,30 +401,59 @@ export class NanoBlockService {
     return block; // return signed block (with or without work)
   }
 
-  async validateAccount(accountInfo) {
+  async validateAccount(accountInfoUntrusted, accountPublicKey) {
-    if (!accountInfo) return;
+    if (!accountInfoUntrusted) return;
-    if (!accountInfo.frontier || accountInfo.frontier === this.zeroHash) {
+
-      if (accountInfo.balance && accountInfo.balance !== '0') {
+    if (!accountInfoUntrusted.frontier || accountInfoUntrusted.frontier === this.zeroHash) {
+      if (accountInfoUntrusted.balance && accountInfoUntrusted.balance !== '0') {
         throw new Error(`Frontier not set, but existing account balance is nonzero`);
       }
-      if (accountInfo.representative) {
+
+      if (accountInfoUntrusted.representative) {
         throw new Error(`Frontier not set, but existing account representative is set`);
       }
+
       return;
     }
-    const blockResponse = await this.api.blocksInfo([accountInfo.frontier]);
+
-    const blockData = blockResponse.blocks[accountInfo.frontier];
+    const frontierBlockResponseUntrusted =
-    if (!blockData) throw new Error(`Unable to load block data`);
+      await this.api.blocksInfo([ accountInfoUntrusted.frontier ]);
-    blockData.contents = JSON.parse(blockData.contents);
+
-    if (accountInfo.balance !== blockData.contents.balance || accountInfo.representative !== blockData.contents.representative) {
+    const frontierBlockDataUntrusted =
+      frontierBlockResponseUntrusted.blocks[accountInfoUntrusted.frontier];
+
+    if (!frontierBlockDataUntrusted) throw new Error(`Unable to load frontier block data`);
+
+    frontierBlockDataUntrusted.contents = JSON.parse(frontierBlockDataUntrusted.contents);
+
+    const isFrontierBlockMatchingAccountInfo = (
+        (frontierBlockDataUntrusted.contents.balance === accountInfoUntrusted.balance)
+      && (frontierBlockDataUntrusted.contents.representative === accountInfoUntrusted.representative)
+    );
+
+    if (isFrontierBlockMatchingAccountInfo !== true) {
       throw new Error(`Frontier block data doesn't match account info`);
     }
-    if (blockData.contents.type !== 'state') {
+
+    if (frontierBlockDataUntrusted.contents.type !== 'state') {
       throw new Error(`Frontier block wasn't a state block, which shouldn't be possible`);
     }
-    if (this.util.hex.fromUint8(this.util.nano.hashStateBlock(blockData.contents)) !== accountInfo.frontier) {
+
+    const isComputedBlockHashMatchingAccountFrontierHash = (
+        this.util.hex.fromUint8( this.util.nano.hashStateBlock(frontierBlockDataUntrusted.contents) )
+      === accountInfoUntrusted.frontier
+    );
+
+    if (isComputedBlockHashMatchingAccountFrontierHash !== true) {
       throw new Error(`Frontier hash didn't match block data`);
     }
+
+    const isFrontierBlockSignatureValid =
+      nanocurrencyWebTools.verifyBlock(accountPublicKey, frontierBlockDataUntrusted.contents);
+
+    if (isFrontierBlockSignatureValid !== true) {
+      throw new Error(`Node provided an untrusted frontier block that was signed by someone else`);
+    }
   }
 
   // Sign a state block, and insert the signature into the block.