refactor: remove unused lambdas #155

andreabadesso · 2024-04-03T16:13:25Z

Motivation

After the refactor of the wallet-service, the daemon is now in charge of accessing the database directly and storing the blockchain state and the lambdas related to receiving transactions should be removed

Acceptance Criteria

Remove all lambdas from the txProcessor, except for the ones dealing with NFTs
Remove all related tests

Checklist

If you are requesting a merge into master, confirm this code is production-ready and can be included in future releases as soon as it gets merged
Make sure either the unit tests and/or the QA tests are capable of testing the new features
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.

andreabadesso · 2024-04-15T00:34:44Z

packages/wallet-service/tests/txProcessor.test.ts

-  await expect(checkAddressBalanceTable(mysql, 4, 'address2', '00', 0, blockReward, null, 1)).resolves.toBe(true);
-});
-
-test.skip('txProcessor should be able to re-process txs that were voided in the past', async () => {


In the new daemon we don't need to re-process transactions, we now handle the metadataChanged event and void the utxos as needed. We have tests for this in packages/daemon/__tests__/services/services.test.ts

andreabadesso · 2024-04-15T01:00:27Z

packages/wallet-service/tests/integration.test.ts

@@ -152,44 +152,6 @@ afterAll(async () => {
  process.env = OLD_ENV;
 });

-// eslint-disable-next-line jest/prefer-expect-assertions, jest/expect-expect
-test('receive blocks and txs and then start wallet', async () => {


We don't have tests for this, need to create tests that use both the lambdas and the daemon

andreabadesso · 2024-04-15T01:00:30Z

packages/wallet-service/tests/integration.test.ts

@@ -248,274 +210,3 @@ test('load wallet, and simulate DLQ event', async () => {
    expect.any(Logger),
  );
 }, 60000);
-
-// eslint-disable-next-line jest/prefer-expect-assertions, jest/expect-expect
-test('start wallet and then receive blocks and txs', async () => {


We don't have tests for this, need to create tests that use both the lambdas and the daemon

andreabadesso · 2024-04-15T01:00:37Z

packages/wallet-service/tests/integration.test.ts

-}, 60000);
-
-// eslint-disable-next-line jest/prefer-expect-assertions, jest/expect-expect
-test('receive blocks, start wallet and then receive transactions', async () => {


We don't have tests for this, need to create tests that use both the lambdas and the daemon

andreabadesso · 2024-04-15T01:00:40Z

packages/wallet-service/tests/integration.test.ts

-}, 35000);
-
-// eslint-disable-next-line jest/prefer-expect-assertions, jest/expect-expect
-test('receive blocks and tx1, start wallet and then receive tx2', async () => {


We don't have tests for this, need to create tests that use both the lambdas and the daemon

andreabadesso · 2024-04-15T01:03:43Z

packages/wallet-service/tests/integration.test.ts

-}, 35000);
-
-// eslint-disable-next-line jest/prefer-expect-assertions, jest/expect-expect
-test('receive blocks fom 3 different miners, check miners list', async () => {


We are not testing this in the new daemon, we should

andreabadesso · 2024-04-15T01:04:02Z

packages/wallet-service/tests/txProcessor.test.ts

-/*
- * In an unlikely scenario, we can receive a tx spending a UTXO that is still marked as locked.
- */
-test.skip('spend "locked" utxo', async () => {


We're handling transactions differently, we receive them in order, so this will never happen

andreabadesso · 2024-04-15T01:04:08Z

packages/wallet-service/tests/txProcessor.test.ts

-  await expect(checkWalletBalanceTable(mysql, 1, walletId, token, 2000, 0, null, 2)).resolves.toBe(true);
-});
-
-test.skip('Genesis transactions should throw', async () => {


We now handle transactions differently, this test has no use anymore

andreabadesso · 2024-04-15T01:04:15Z

packages/wallet-service/tests/txProcessor.test.ts

-/*
- * receive some transactions and blocks and make sure database is correct
- */
-test.skip('txProcessor', async () => {


We have integration tests that run the simulation in the fullnode and check if balances are correct after a full sync which is way better than mocking it. So no need to re-implement this test.

andreabadesso · 2024-04-15T01:04:28Z

packages/wallet-service/tests/txProcessor.test.ts

-  )).toStrictEqual(true);
-});
-
-test.skip('txProcessor should ignore NFT outputs', async () => {


We are still ignoring NFT outputs, but have no tests for this case, we should add it

andreabadesso · 2024-04-15T01:04:37Z

packages/wallet-service/tests/txProcessor.test.ts

@@ -515,300 +93,3 @@ describe('NFT metadata updating', () => {
    spyCreateOrUpdate.mockRestore();
  });
 });
-
-test.skip('receive token creation tx', async () => {


We also need to add this one, we're adding token information into the tokens table, but not testing it.

Maybe an integration test with token creation simulations.

andreabadesso · 2024-04-15T01:04:44Z

packages/wallet-service/tests/txProcessor.test.ts

-  expect(tokenInfo.symbol).toBe(tokenCreationTx.token_symbol);
-});
-
-test.skip('onHandleVoidedTxRequest', async () => {


We handle voided transactions differently now, this lambda is dead.

andreabadesso · 2024-04-15T01:05:59Z

packages/wallet-service/tests/txProcessor.test.ts

-  await expect(checkAddressBalanceTable(mysql, 2, addr, token, 2500, 0, null, 1)).resolves.toBe(true);
-}, 20000);
-
-test.skip('txProcessor should rollback the entire transaction if an error occurs on balance calculation', async () => {


We are still rolling back when we fail to process a message, but now on different methods — handleVoidedTx, handleVertexAccepted, handleUnvoidedTx

We should re-create this test for all of them

andreabadesso · 2024-04-15T01:06:22Z

packages/wallet-service/tests/txProcessor.test.ts

-  await expect(checkAddressBalanceTable(mysql, 1, 'address1', '00', blockReward * 4, blockReward, null, 5)).resolves.toBe(true);
-});
-
-test.skip('txProcess onNewTxRequest with push notification', async () => {


The daemon now simply calls the push notification lambda, but we don't have tests for it, we should add it

andreabadesso · 2024-04-15T01:06:28Z

packages/wallet-service/tests/txProcessor.test.ts

-  expect(invokeOnTxPushNotificationRequestedLambdaMock).toHaveBeenCalledTimes(1);
-});
-
-test.skip('onNewTxRequest should send alert on SQS on failure', async () => {


We are dealing with events differently now, but we're not sending an alert with rich metadata when the daemon stops. We should refactor the daemon to add an alert directly to SQS (instead of just logging the error) and add a test for it.

…cessor

* feat: added nft utils * tests: added tests for common utils * chore: added common module to CI * refactor: moved used types to common package * tests: removed nft utils * tests: fixed nft tests on txProcessor * tests: mocking network on getconfig * tests: fixed nft tests on txProcessor * refactor: passing logger to invoke nft handler * refactor: no need to ignore ts * chore: removed jest script, instead using only test * chore: added hathor header * refactor: using common utils on txProcessor * tests: nft utils using old syntax * tests: skipped txProcessor tests * tests: fixed txProcessor tests * refactor: using isAuthority from common utils * refactor: using isAuthority from common types * refactor: using assertEnvVariablesExistance from common project * refactor: getting CREATE_NFT_MAX_RETRIES from env * docs: updated docstrings on nft utils * chore: removed events/nftCreationTx.ts * refactor: invalid import locations * refactor: remove unused lambdas (#155) * tests: fixed nft tests on txProcessor * refactor: removed all methods related to the old wallet-service txProcessor

* chore: added common package * chore: using wallet-lib from daemon resolution * chore: installed shared dependencies on root project, using peerDependencies * refactor: using addAlert method from common utils * chore: updated package.json with missing deps * refactor: using types from commons on wallet-service * chore: re-added sequelize to root * refactor: removed isNftAutoReviewEnabled from services * tests: mocked assertEnv * tests: mocked assertEnv on integration tests * chore: removed nft utils * refactor: removed old addAlerts from the wallet-service package * chore: wallet-lib is now a peerDependency in wallet-service package * refactor: logger is now a required param in addAlert, refactored all methods in wallet-service package * docs: added missing hathor header * refactor: invalid type for metadata on addAlert * tests: passing mocked logger * chore: updated gitignore to ignore env files * chore: revert eslint package updates * refactor: using types from commons on wallet-service * refactor: removed unused commented line * chore: missing package in daemon * refactor: using transaction type from common * refactor: bitcoinjs is not a shared dep * chore: added a comment explaining that logger is a param temporarily * refactor: removed default from metadata * chore: lint fixes and package ordering * feat: call onNewNftEvent when a new NFT tx is received (#150) * feat: added nft utils * tests: added tests for common utils * chore: added common module to CI * refactor: moved used types to common package * tests: removed nft utils * tests: fixed nft tests on txProcessor * tests: mocking network on getconfig * tests: fixed nft tests on txProcessor * refactor: passing logger to invoke nft handler * refactor: no need to ignore ts * chore: removed jest script, instead using only test * chore: added hathor header * refactor: using common utils on txProcessor * tests: nft utils using old syntax * tests: skipped txProcessor tests * tests: fixed txProcessor tests * refactor: using isAuthority from common utils * refactor: using isAuthority from common types * refactor: using assertEnvVariablesExistance from common project * refactor: getting CREATE_NFT_MAX_RETRIES from env * docs: updated docstrings on nft utils * chore: removed events/nftCreationTx.ts * refactor: invalid import locations * refactor: remove unused lambdas (#155) * tests: fixed nft tests on txProcessor * refactor: removed all methods related to the old wallet-service txProcessor * tests: fixed failing test * fix: serverless failing (#162) * refactor: logger is now a required param in addAlert, refactored all methods in wallet-service package * chore: changed module resolution and several package locks * chore: added common library to externals whitelist and whitelisted it in tsloader * chore: wallet-service common package moved from direct path to workspace * chore: added common as a peer dependency and restored peer dependencies * chore: removed peer dependencies as they were not working with serverless-monorepo * chore: added comment on hoisting limits * chore: added comment explaining externals in webpack

andreabadesso requested review from tuliomir and pedroferreira1 April 3, 2024 16:13

andreabadesso self-assigned this Apr 3, 2024

andreabadesso added the enhancement New feature or request label Apr 3, 2024

tuliomir approved these changes Apr 3, 2024

View reviewed changes

andreabadesso force-pushed the feat/send-nft branch 3 times, most recently from 73e4c9f to cd595af Compare April 8, 2024 18:56

andreabadesso force-pushed the refactor/remove-unused-lambdas branch from 37f1d0b to 3dbe90c Compare April 8, 2024 19:16

andreabadesso force-pushed the feat/send-nft branch from 5f652ad to f047ece Compare April 9, 2024 15:08

andreabadesso force-pushed the refactor/remove-unused-lambdas branch from 3dbe90c to 7e3603c Compare April 9, 2024 15:09

andreabadesso commented Apr 15, 2024

View reviewed changes

pedroferreira1 approved these changes Apr 18, 2024

View reviewed changes

andreabadesso force-pushed the feat/send-nft branch 2 times, most recently from 0de6578 to a951693 Compare April 19, 2024 20:46

tests: fixed nft tests on txProcessor

e32c63d

refactor: removed all methods related to the old wallet-service txPro…

3779b1e

…cessor

andreabadesso force-pushed the refactor/remove-unused-lambdas branch from 7e3603c to 3779b1e Compare April 19, 2024 22:16

andreabadesso merged commit b6788ad into feat/send-nft Apr 19, 2024
1 check passed

andreabadesso deleted the refactor/remove-unused-lambdas branch April 19, 2024 22:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: remove unused lambdas #155

refactor: remove unused lambdas #155

andreabadesso commented Apr 3, 2024 •

edited

Loading

andreabadesso Apr 15, 2024

andreabadesso Apr 15, 2024

andreabadesso Apr 15, 2024

andreabadesso Apr 15, 2024

andreabadesso Apr 15, 2024

andreabadesso Apr 15, 2024

andreabadesso Apr 15, 2024

andreabadesso Apr 15, 2024

andreabadesso Apr 15, 2024

andreabadesso Apr 15, 2024

andreabadesso Apr 15, 2024

andreabadesso Apr 15, 2024

andreabadesso Apr 15, 2024

andreabadesso Apr 15, 2024

andreabadesso Apr 15, 2024

refactor: remove unused lambdas #155

refactor: remove unused lambdas #155

Conversation

andreabadesso commented Apr 3, 2024 • edited Loading

Motivation

Acceptance Criteria

Checklist

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

andreabadesso commented Apr 3, 2024 •

edited

Loading