feat: persistent storage #462

r4mmer · 2023-01-24T01:12:19Z

Acceptance Criteria

Implement a store of data using LevelDB
Implement reverse search indexes to comply with the IStore interface
Deprecate node v8 and v10

Security Checklist

Make sure you do not include new dependencies in the project unless strictly necessary and do not include dev-dependencies as production ones. More dependencies increase the possibility of one of them being hijacked and affecting us.

codecov · 2023-03-06T14:36:41Z

Codecov Report

Merging #462 (49f4f4a) into feat/new-storage-scheme (9a7ddd5) will increase coverage by 0.86%.
The diff coverage is 64.76%.

❗ Current head 49f4f4a differs from pull request most recent head 9e72320. Consider uploading reports for the commit 9e72320 to get more accurate results

@@                     Coverage Diff                     @@
##           feat/new-storage-scheme     #462      +/-   ##
===========================================================
+ Coverage                    52.04%   52.90%   +0.86%     
===========================================================
  Files                           57       64       +7     
  Lines                         4206     4682     +476     
  Branches                       868      934      +66     
===========================================================
+ Hits                          2189     2477     +288     
- Misses                        1923     2111     +188     
  Partials                        94       94

Impacted Files	Coverage Δ
src/api/metadataApi.ts	`0.00% <0.00%> (ø)`
src/new/wallet.js	`27.32% <0.00%> (-0.05%)`	⬇️
src/storage/index.ts	`100.00% <ø> (ø)`
src/types.ts	`33.33% <ø> (ø)`
src/utils/tokens.ts	`26.36% <40.00%> (-2.47%)`	⬇️
src/storage/leveldb/utxo_index.ts	`48.14% <48.14%> (ø)`
src/storage/leveldb/errors.ts	`50.00% <50.00%> (ø)`
src/storage/leveldb/history_index.ts	`56.25% <56.25%> (ø)`
src/storage/leveldb/token_index.ts	`57.14% <57.14%> (ø)`
src/storage/leveldb/address_index.ts	`57.30% <57.30%> (ø)`
... and 4 more

... and 2 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

r4mmer · 2023-03-06T19:38:58Z

.github/workflows/integration-test.yml

@@ -4,6 +4,7 @@ on:
    branches:
      - dev
      - master
+      - feat/new-storage-scheme


This is to make the CI run on the PR, it will be removed before being merged.

r4mmer · 2023-03-06T19:39:16Z

.github/workflows/main.yml

@@ -10,13 +10,14 @@ on:
    branches:
    - dev
    - master
+    - feat/new-storage-scheme


This is to make the CI run on the PR, it will be removed before being merged.

andreabadesso · 2023-03-08T13:52:14Z

__tests__/integration/storage/leveldb.test.js

+  }
+}
+
+describe("LevelDB persistent store", () => {


Use single quotes

andreabadesso · 2023-03-09T20:19:44Z

__tests__/storage/leveldb/leveldb_store.test.js

+  }
+  // The order should follow the index ordering
+  expect(values).toStrictEqual(['b', 'a', 'c']);
+


Extra blankline

andreabadesso · 2023-03-09T20:45:47Z

index.js

@@ -9,7 +9,9 @@ const txMiningApi = require('./lib/api/txMining');
 const versionApi = require('./lib/api/version');
 const axios = require('./lib/api/axiosInstance');
 const metadataApi = require('./lib/api/metadataApi');
-const storage = require('./lib/storage');
+const storage = require('./lib/storage/storage');


Suggested change

const storage = require('./lib/storage/storage');

const { Storage } = require('./lib/storage/storage');

andreabadesso · 2023-03-09T20:46:13Z

index.js

@@ -57,7 +59,9 @@ module.exports = {
  ErrorMessages: ErrorMessages,
  constants,
  axios,
-  storage: storage.default,
+  Storage: storage.Storage,


Suggested change

Storage: storage.Storage,

Storage,

andreabadesso · 2023-03-09T20:46:37Z

setupTests.js

@@ -72,4 +72,9 @@ expect.extend({
 });


-global.window = {};
+// global.window = {};


andreabadesso · 2023-03-09T21:22:40Z

src/storage/leveldb/history_index.ts

+
+      try {
+        await this.tsHistoryDB.get(`${value.timestamp}:${value.tx_id}`);
+      } catch(err: unknown) {


Suggested change

} catch(err: unknown) {

} catch (err: unknown) {

andreabadesso · 2023-03-09T21:25:49Z

src/storage/leveldb/store.ts

+    if (process.version < 'v14.14.0') {
+      // rmSync was introduced in v14.14.0, earlier versions should use rmdirSync
+      // https://nodejs.org/docs/latest-v14.x/api/fs.html#fs_fs_rmdirsync_path_options
+      fs.rmdirSync(this.dbpath, { recursive: true });
+    } else {
+      fs.rmSync(this.dbpath, { recursive: true, force: true });
+    }


This method already returns a promise, why do we need to use the sync methods?

This will only cause clients using our lib and calling destroy() to be slower as it will block the event loop

Started using async version or rm

andreabadesso · 2023-03-09T21:27:36Z

src/storage/leveldb/store.ts

+
+  async saveAddress(info: IAddressInfo): Promise<void> {
+    if (!info.base58) {
+      throw new Error('Invalid address');


Should we have standard errors like the ones in errors.ts so they can be better handled?

e.g.

class InvalidAddressError extends Error

I also believe we should create a standard error, but since it would affect both the persistent storage files and the base storage scheme implementation I think we should make this on its own PR, what do you think?

I agree, can you create an issue so we don't forget to refactor this?

src/storage/leveldb/store.ts

src/storage/leveldb/utxo_index.ts

setupTests.js

src/storage/memory_store.ts

src/storage/leveldb/store.ts

src/storage/leveldb/address_index.ts

src/storage/leveldb/token_index.ts

src/storage/leveldb/utxo_index.ts

src/storage/leveldb/wallet_index.ts

…g every call

src/storage/leveldb/address_index.ts

src/storage/leveldb/history_index.ts

pedroferreira1 · 2023-03-16T23:42:41Z

src/storage/leveldb/history_index.ts

  }

  /**
   * Run a full count of the txs in the database.
-   * Since we have made a full count we can update the tx count in the database.


You could also remove this line in the address index file.

Removed from the other file

* feat/new-storage-scheme: chore: rename methods

r4mmer requested a review from pedroferreira1 as a code owner January 24, 2023 01:12

r4mmer self-assigned this Jan 24, 2023

r4mmer changed the base branch from dev to feat/new-storage-scheme January 24, 2023 01:12

r4mmer force-pushed the feat/persistent-storage branch from 2b0f792 to 378368e Compare March 3, 2023 16:56

r4mmer requested a review from andreabadesso March 3, 2023 16:56

r4mmer commented Mar 6, 2023

View reviewed changes

andreabadesso requested changes Mar 9, 2023

View reviewed changes

r4mmer force-pushed the feat/persistent-storage branch from 00a8d5d to 4fcf868 Compare March 10, 2023 14:31

pedroferreira1 reviewed Mar 10, 2023

View reviewed changes

r4mmer requested review from andreabadesso and pedroferreira1 March 10, 2023 21:50

r4mmer added 17 commits March 15, 2023 12:05

feat: persistent storage with leveldb

c827744

tests: leveldb store tests

3896d9a

chore: temporary add ci on base feat/new-storage-scheme

28b9581

chore: update typescript

729a976

use rmdirSync on node < v14.14.0

1f96ad8

chore: remove node version 8 and 10 from ci

683a90d

chore: remove v16 from ci

6519dba

tests(integration): persistent storage

272c0f9

chore: review changes

dcc89f5

feat: await rmdir

a2c507c

chore: remove blank line

ead3396

chore: review changes

a5a49ca

chore: remove unused generic db access

85336a7

chore: add htr to tokens on constructor

1b2bee9

chore: split nft creation fee from mint deposit

2359a8c

chore: await async methods

57385a3

feat: process history using the storage util

66121af

r4mmer force-pushed the feat/persistent-storage branch from 920957d to 66121af Compare March 15, 2023 15:28

r4mmer added 5 commits March 15, 2023 13:00

feat: save address and history count on storage instead of calculatin…

f60f5e0

…g every call

chore: add delays

6575bbf

feat: only process walletData if there is something to process

4be0f8d

chore: integration test delay funds

9be45fb

feat: actually increment counter when adding addresses

8b8d37c

pedroferreira1 reviewed Mar 16, 2023

View reviewed changes

src/storage/leveldb/address_index.ts Outdated Show resolved Hide resolved

src/storage/leveldb/history_index.ts Outdated Show resolved Hide resolved

feat: save the index size on memory

49f4f4a

r4mmer requested a review from pedroferreira1 March 16, 2023 17:47

pedroferreira1 approved these changes Mar 16, 2023

View reviewed changes

r4mmer added 3 commits March 16, 2023 21:12

chore: remove changes to force CI

b41cce1

Merge branch 'feat/new-storage-scheme' into feat/persistent-storage

c5f3d71

* feat/new-storage-scheme: chore: rename methods

chore: rename editToken and editAddress methods for leveldb store

9e72320

andreabadesso approved these changes Mar 17, 2023

View reviewed changes

r4mmer merged commit b72e950 into feat/new-storage-scheme Mar 17, 2023

r4mmer deleted the feat/persistent-storage branch March 17, 2023 03:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: persistent storage #462

feat: persistent storage #462

r4mmer commented Jan 24, 2023 •

edited

Loading

codecov bot commented Mar 6, 2023 •

edited

Loading

r4mmer Mar 6, 2023

r4mmer Mar 6, 2023

andreabadesso Mar 8, 2023

r4mmer Mar 10, 2023

andreabadesso Mar 9, 2023

r4mmer Mar 10, 2023

andreabadesso Mar 9, 2023

r4mmer Mar 10, 2023

andreabadesso Mar 9, 2023

r4mmer Mar 10, 2023

andreabadesso Mar 9, 2023

r4mmer Mar 10, 2023

andreabadesso Mar 9, 2023

r4mmer Mar 10, 2023

andreabadesso Mar 9, 2023

r4mmer Mar 10, 2023

andreabadesso Mar 9, 2023

r4mmer Mar 10, 2023

andreabadesso Mar 17, 2023

pedroferreira1 Mar 16, 2023

r4mmer Mar 17, 2023

	const storage = require('./lib/storage/storage');
	const { Storage } = require('./lib/storage/storage');

feat: persistent storage #462

feat: persistent storage #462

Conversation

r4mmer commented Jan 24, 2023 • edited Loading

Acceptance Criteria

Security Checklist

codecov bot commented Mar 6, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

r4mmer commented Jan 24, 2023 •

edited

Loading

codecov bot commented Mar 6, 2023 •

edited

Loading