-
Notifications
You must be signed in to change notification settings - Fork 0
New INTERX support with Blocks & Transactions list features #31
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
base: dev
Are you sure you want to change the base?
Conversation
…sactions page Those changes are only the beginning for the TransactionsPage, and I don't recommend to review them carefully without making a notice of the further implementation of the TransactionsPage. Renamed specific pages, widgets, made models more convenient to use in one place. List of changes: - renamed Transaction... classes to MyTransaction..., cause they are part of MyAccountPage, and there is a separate TransactionsPage coming soon - made ATxMsgModel as sealed class, so every programmer can iterate through all possible models using switch-case - added common component StatusChip, replaced usages for all status chips
Added TransactionsPage, TransactionDetailsDrawerPage. Slightly fixed workaround of the components such as ListPopMenu, FilterDropdown for other list pages (validators, my-transactions). List of changes: - added TransactionsPage for all transactions with ability to search by hash/from/to fields, to filter by date and method. On row tap it will transfer to details drawer. - added TransactionDetailsDrawerPage with details for each transaction method. The common details at the beginning, the specific - at the bottom. - added fromAddress and toAddress to ATxMsgModel to make able to display it for each child model (default is null) - made ListPopMenu with dynamic width but with minimal as 150px, so we can pass long texts without calculating the width manually - fixed FilterDropdown width adjusting. It was not working before because of infinite width in Row.
Added BlocksPage and BlockDetailsPage UI together with fetching logic through controller and API. Optimized TransactionsListController and TransactionsPage to use in both blocks and transactions List of changes: - added BlocksPage(mobile + desktop) - added BlockDetailsPage which opens by tap on block item - extracted content of TransactionsPage into separate TransactionsPageSliver widget to reuse in blocks - moved detail key-value widgets into separate files - added blockModel as param to TransactionsListController, so txs can be fetched by block id, and avoid fetch if no txs inside - added API for blocks - fixed types of BlockModel
… of the full versions.
…rs with new Interx API
…ewly added Proposals, and other existing endpoints
MrLutik
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review: PR #31 - New INTERX support with Blocks & Transactions list features
Summary
This PR introduces significant new functionality for INTERX API support with blocks and transactions listing. However, several critical bugs need to be addressed before merging.
Verdict: REQUEST CHANGES
Critical Issues (Must Fix)
1. Missing Query Parameters in Block Transactions API
File: lib/infra/repositories/api/api_repository.dart:101-114
The fetchQueryBlockTransactions() method does not pass query parameters to the HTTP request:
path: '/api/blocks/${apiRequestModel.requestData.blockId}/transactions',
// MISSING: queryParameters: apiRequestModel.requestData.toJson(),Impact: All block transaction filtering (by address, date, status, type) is completely non-functional.
Fix: Add queryParameters: apiRequestModel.requestData.toJson() to the HTTP GET call.
2. Wrong Transaction Message Type
File: lib/shared/models/transactions/messages/staking_msg_claim_undelegation_model.dart:10
StakingMsgClaimUndelegationModel({...})
: super(txMsgType: TxMsgType.msgClaimRewards); // Should be msgClaimUndelegationImpact: Claim undelegation transactions are incorrectly identified as claim rewards, breaking filtering, routing, and analytics.
3. String Comparison for Block Height (Sorting Bug)
File: lib/shared/controllers/menu/blocks_page/blocks_sort_options.dart:8
comparator: (BlockModel a, BlockModel b) =>
a.header.height.compareTo(b.header.height), // String comparison!Impact: Block sorting is lexicographic, not numeric. Block 9 sorts after block 100.
Fix:
comparator: (BlockModel a, BlockModel b) =>
int.parse(a.header.height).compareTo(int.parse(b.header.height)),4. Wrong Dashboard Statistics Calculation
File: lib/shared/models/dashboard/dashboard_model.dart:102-103
for (ProposalModel proposal in proposalsResp.proposals) {
proposersSet.add(proposal.proposalId); // Should be proposal.proposer
votersSet.add(proposal.proposalId); // Should be proposal.voter
}Impact: Dashboard shows incorrect unique proposers/voters count.
5. Unsafe Type Casting in DTOs
File: lib/infra/dto/api_kira/broadcast/response/broadcast_resp.dart:18-26
hash: json['hash'] as String, // Crashes if null
code: json['code'] as int, // Crashes if nullAlso affects:
lib/infra/dto/api/query_blocks_transactions/response/query_block_transactions_resp.dart:18lib/infra/dto/api_kira/broadcast/response/event.dart:15
Fix: Add null safety: json['hash'] as String? ?? ''
Major Issues (Should Fix)
6. Memory Leak - TxFormBuilderCubit Never Closed
File: lib/blocs/pages/transactions/tx_process_cubit/tx_process_cubit.dart:115-123
TxFormBuilderCubit is instantiated but never closed, causing memory leaks.
7. Empty Equatable Props
File: lib/blocs/pages/transactions/tx_broadcast/a_tx_broadcast_state.dart:6
List<Object?> get props => <Object>[]; // Empty!All child states compare as equal regardless of content. BLoC won't emit state changes properly.
8. Mutable State in FavouritesBloc
File: lib/blocs/widgets/kira/kira_list/favourites/favourites_bloc.dart:39-50
item.favourite = true; // Direct mutation violates immutability9. copyWith Result Discarded
File: lib/shared/controllers/menu/transactions_page/transactions_list_controller.dart:62
})
..copyWith(blockDateTime: blockModel?.header.time); // Result lost!Cascade operator doesn't return value; the modification is lost.
10. Missing undelegationId in Props
File: lib/shared/models/transactions/messages/staking_msg_claim_undelegation_model.dart:28
List<Object?> get props => <Object>[senderWalletAddress]; // Missing undelegationId!11. Wrong Error Log Message
File: lib/infra/repositories/api/api_repository.dart:111
Error message says fetchQueryBlocks() but is inside fetchQueryBlockTransactions().
Moderate Issues
12. Dead Code
File: lib/config/app_config.dart:75
bool isInterxVersionOutdated(String version) {
// TODO: #22
return true; // Dead code below never executes
bool isVersionSupported = ...13. Unresolved TODOs in Production
- TODO #27:
dashboard_model.dart:128- Transactions always 0 - TODO #29:
list_loaded_state.dart:12- Cache expiration - TODO #32:
block_model.dart:35- blockSize not available
Positive Observations
- Consistent BLoC pattern usage
- Good separation of concerns
- Proper defensive coding for API migration with fallbacks
- Clean service locator pattern
Recommendation
Please address the 5 critical issues before merging. The major issues can be tracked as follow-up tasks but ideally should be fixed in this PR given they affect core functionality.
…y params (BlockTx); wrong tx type (msgClaimUndelegation); make height operate in int (no str sorting); voter count fix
|
@MrLutik 1-4, 10-11 fixed
12-13 will be fixed in the nearest future PRs |
No description provided.