Skip to content
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

chore: sort completed transactions #6733

Merged

Conversation

mmrrnn
Copy link
Contributor

@mmrrnn mmrrnn commented Jan 7, 2025

Description

This pull request focuses on refactoring the transaction handling in the wallet service by replacing HashMap with Vec for storing transactions. This change impacts several parts of the codebase, including transaction service responses, database storage, and transaction handling logic. Additionaly, we sort txs by mined_timestamp in desc order.

Testing

Tested manually using minotari_console_wallet

image

@mmrrnn mmrrnn marked this pull request as draft January 7, 2025 14:57
Copy link

github-actions bot commented Jan 7, 2025

Test Results (CI)

1 349 tests   1 348 ✅  12m 7s ⏱️
   43 suites      0 💤
    1 files        1 ❌

For more details on these failures, see this check.

Results for commit bec825e.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Jan 7, 2025

Test Results (Integration tests)

36 tests  +36   36 ✅ +36   22m 29s ⏱️ + 22m 29s
11 suites +11    0 💤 ± 0 
 2 files   + 2    0 ❌ ± 0 

Results for commit bec825e. ± Comparison against base commit c7a423d.

♻️ This comment has been updated with latest results.

Copy link
Collaborator

@SWvheerden SWvheerden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not do an order here in rust.
I would change it where it gets it from the database and use SQL to to do the order.

Also don't order by timestamp, order by mined_timestamp

@mmrrnn mmrrnn marked this pull request as ready for review January 16, 2025 10:19
@mmrrnn mmrrnn requested a review from a team as a code owner January 16, 2025 10:19
Copy link
Collaborator

@SWvheerden SWvheerden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@SWvheerden SWvheerden merged commit e5193a8 into tari-project:development Jan 17, 2025
16 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants