Hotfix/account balances main #757
Conversation
WalkthroughThe recent update in the Changes
Poem
Recent Review DetailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Additional comments not posted (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| @@ -19,6 +19,7 @@ import { | |||
| QueryAccountRequest, | |||
| QueryAccountResponse, | |||
| } from '@aura-nw/aurajs/types/codegen/cosmos/auth/v1beta1/query'; | |||
| import _ from 'lodash'; | |||
There was a problem hiding this comment.
Consider using a more specific import for lodash to reduce bundle size.
Instead of importing the entire lodash library, consider importing only the functions you need. This can help reduce the overall bundle size and improve the application's performance. For example:
import keyBy from 'lodash/keyBy';| last_updated_height: number; | ||
| }[] = []; | ||
| await knex.transaction(async (trx) => { | ||
| const addressesWithIds = await Account.query() | ||
| .select('id', 'address') | ||
| .forUpdate() | ||
| .whereIn('address', addresses) | ||
| .transacting(trx); | ||
| const addressesWithIds = _.keyBy( | ||
| await Account.query() | ||
| .select('id', 'address') | ||
| .forUpdate() | ||
| .whereIn('address', addresses) | ||
| .transacting(trx), | ||
| 'address' | ||
| ); | ||
| const resetBalanceList: { | ||
| account_id: number; | ||
| last_updated_height: number; | ||
| }[] = accounts | ||
| .filter((account) => addressesWithIds[account.address]) | ||
| .map((account) => ({ | ||
| account_id: addressesWithIds[account.address].id, | ||
| last_updated_height: account.last_updated_height, | ||
| })); | ||
| accounts.forEach((account) => { | ||
| const accountId = addressesWithIds.find( | ||
| (addressWithId) => addressWithId.address === account.address | ||
| )?.id; | ||
| if (Array.isArray(account.balances) && accountId) | ||
| const accountId = addressesWithIds[account.address]?.id; | ||
| if (Array.isArray(account.balances) && accountId) { | ||
| account.balances.forEach((balance) => { | ||
| listAccountBalance.push({ | ||
| account_id: accountId, |
There was a problem hiding this comment.
📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [306-347]
Ensure transactional integrity and handle potential race conditions.
The transaction block from lines 306 to 347 updates account balances and inserts new records. However, there are a few concerns:
- The use of raw SQL queries (lines 344-346 and subsequent lines) can be prone to SQL injection if not properly handled. Always ensure that parameters are properly sanitized.
- The transaction does not seem to handle potential race conditions between read and write operations. Consider using row-level locking mechanisms if applicable.
- The error handling within the transaction only logs the error but does not rethrow it, potentially leading to silent failures in some scenarios. Consider rethrowing the error or handling it in a way that the calling function is aware of the failure.
Summary by CodeRabbit