Skip to content

Conversation

@gfyrag
Copy link
Contributor

@gfyrag gfyrag commented Mar 20, 2025

No description provided.

@gfyrag gfyrag requested a review from a team as a code owner March 20, 2025 08:09
@coderabbitai
Copy link

coderabbitai bot commented Mar 20, 2025

Walkthrough

This pull request updates the transaction commit process across multiple components. The changes add an extra parameter—either an account metadata map or a placeholder value—to calls of the CommitTransaction method. As a consequence, both production code and tests in the controller and storage layers have been adjusted to match the new signature, and redundant metadata update logic has been removed.

Changes

File(s) Change Summary
internal/controller/ledger/{controller_default.go, controller_default_test.go} Modified methods (e.g., importLog, createTransaction, revertTransaction) to pass an AccountMetadata parameter in CommitTransaction; removed explicit metadata length checks and update logic; updated tests accordingly.
internal/controller/ledger/{store.go, store_generated_test.go} Updated the CommitTransaction method in the Store interface and associated mocks to accept an additional accountMetadata parameter; adjusted method calls and test recorders.
internal/storage/ledger/{accounts_test.go, balances_test.go, moves_test.go, transactions.go, transactions_test.go, volumes_test.go} Altered the CommitTransaction signature and its invocations to include a new third parameter (nil, options, or account metadata); revised account metadata assignment in transaction commits and updated corresponding tests.

Sequence Diagram(s)

sequenceDiagram
  participant C as Controller
  participant S as Store
  participant L as Ledger

  C->>S: Call CommitTransaction(tx, accountMetadata)
  S->>L: Process transaction with provided accountMetadata
  L-->>S: Return processing result
  S-->>C: Return final confirmation
Loading

Possibly related PRs

  • fix: import from 2.1 #688: Updates the CommitTransaction signature to include an AccountMetadata parameter, aligning directly with this PR's changes.
  • feat: refactor read store #615: Modifies the DefaultController’s commit calls by adding an extra parameter, indicating a direct connection at the code level.
  • fix: metadata override #734: Adjusts transaction metadata handling via updates to CommitTransaction, paralleling the modifications in this PR.

Suggested labels

build-images

Poem

I'm a hoppy rabbit, leaping with glee,
As metadata flows where it was meant to be.
Transactions now carry a brand-new flair,
Passing account data with extra care.
In code meadows, I hop and I play—
Celebrating smooth commits all day!
🐇✨


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d743757 and a8d8b2f.

📒 Files selected for processing (10)
  • internal/controller/ledger/controller_default.go (4 hunks)
  • internal/controller/ledger/controller_default_test.go (2 hunks)
  • internal/controller/ledger/store.go (1 hunks)
  • internal/controller/ledger/store_generated_test.go (1 hunks)
  • internal/storage/ledger/accounts_test.go (5 hunks)
  • internal/storage/ledger/balances_test.go (2 hunks)
  • internal/storage/ledger/moves_test.go (1 hunks)
  • internal/storage/ledger/transactions.go (2 hunks)
  • internal/storage/ledger/transactions_test.go (17 hunks)
  • internal/storage/ledger/volumes_test.go (2 hunks)
🧰 Additional context used
🧬 Code Definitions (6)
internal/storage/ledger/volumes_test.go (1)
internal/storage/ledger/transactions.go (6) (6)
  • store (37-109)
  • store (111-155)
  • store (158-186)
  • store (188-217)
  • store (219-243)
  • store (245-269)
internal/storage/ledger/balances_test.go (1)
internal/storage/ledger/transactions.go (6) (6)
  • store (37-109)
  • store (111-155)
  • store (158-186)
  • store (188-217)
  • store (219-243)
  • store (245-269)
internal/storage/ledger/accounts_test.go (1)
internal/storage/ledger/transactions.go (6) (6)
  • store (37-109)
  • store (111-155)
  • store (158-186)
  • store (188-217)
  • store (219-243)
  • store (245-269)
internal/storage/ledger/transactions_test.go (1)
internal/storage/ledger/transactions.go (6) (6)
  • store (37-109)
  • store (111-155)
  • store (158-186)
  • store (188-217)
  • store (219-243)
  • store (245-269)
internal/controller/ledger/store_generated_test.go (8)
internal/controller/ledger/controller_default.go (17) (17)
  • ctrl (55-67)
  • ctrl (69-71)
  • ctrl (73-75)
  • ctrl (113-115)
  • ctrl (117-119)
  • ctrl (121-123)
  • ctrl (125-127)
  • ctrl (129-131)
  • ctrl (133-135)
  • ctrl (137-139)
  • ctrl (141-143)
  • ctrl (145-151)
  • ctrl (153-155)
  • ctrl (157-159)
  • ctrl (161-195)
  • ctrl (197-261)
  • _ (489-489)
internal/storage/driver/driver.go (1) (1)
  • ret (43-43)
internal/controller/ledger/store.go (2) (2)
  • _ (133-133)
  • _ (231-231)
internal/controller/ledger/controller_with_traces.go (1) (1)
  • _ (458-458)
internal/controller/ledger/controller_with_too_many_client_handling.go (1) (1)
  • _ (135-135)
internal/controller/ledger/controller_with_events.go (1) (1)
  • _ (188-188)
internal/api/bulking/bulker.go (1) (1)
  • _ (313-313)
internal/storage/ledger/adapters.go (1) (1)
  • _ (52-52)
internal/storage/ledger/transactions.go (3)
internal/storage/ledger/accounts.go (3) (3)
  • store (21-71)
  • store (73-90)
  • store (92-126)
internal/storage/ledger/volumes.go (1) (1)
  • store (11-57)
internal/storage/ledger/store.go (11) (11)
  • store (47-55)
  • store (57-59)
  • store (61-69)
  • store (71-79)
  • store (81-89)
  • store (91-100)
  • store (102-109)
  • store (111-118)
  • store (120-122)
  • store (124-126)
  • Store (23-45)
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Tests
🔇 Additional comments (25)
internal/storage/ledger/balances_test.go (2)

177-177: Updated CommitTransaction call to include an accountMetadata parameter.

The function call is properly updated to match the new signature of the CommitTransaction method, which now accepts an additional accountMetadata parameter. This aligns with the PR objective of optimizing metadata handling during transaction commits.


188-188: Updated CommitTransaction call to include an accountMetadata parameter.

The function call is properly updated to match the new signature of the CommitTransaction method. Passing nil for the metadata parameter is appropriate as the implementation handles this case by initializing an empty map.

internal/storage/ledger/accounts_test.go (7)

34-34: Updated CommitTransaction call with nil metadata parameter.

The call has been properly updated to match the new function signature that includes an additional accountMetadata parameter. This change supports the PR's objective of removing a separate UPDATE SQL query for metadata handling.


58-58: Updated CommitTransaction call with nil metadata parameter.

The function call has been correctly updated to match the new signature, passing nil for the metadata parameter. This is consistent with the changes throughout the codebase.


64-64: Updated CommitTransaction call with nil metadata parameter.

The call has been properly updated to match the new function signature. The change is consistent with the PR objective of optimizing how account metadata is handled.


70-70: Updated CommitTransaction call with nil metadata parameter.

The function call has been correctly updated to match the new signature. This is part of the broader change to streamline transaction metadata handling.


308-308: Updated CommitTransaction call with nil metadata parameter.

The call has been properly updated to match the new function signature. This maintains test functionality while accommodating the API change.


323-323: Updated CommitTransaction call with nil metadata parameter.

The function call has been correctly updated to match the new signature. This change aligns with the refactoring of how transaction metadata is handled.


449-449: Updated CommitTransaction call with nil metadata parameter.

The call has been properly updated to match the new function signature. This ensures the test continues to function correctly with the modified API.

internal/storage/ledger/moves_test.go (1)

160-160: Updated CommitTransaction call to include accountMetadata parameter.

The function call has been properly updated to match the new signature of CommitTransaction, which now accepts an additional accountMetadata parameter. This change is part of the optimization to combine metadata updates with transaction commits, removing the need for a separate UPDATE query.

internal/storage/ledger/volumes_test.go (3)

54-54: Updated CommitTransaction call to include accountMetadata parameter.

The function call has been properly updated to match the new signature of CommitTransaction. This change aligns with the PR objective of optimizing metadata handling during transaction commits.


61-61: Updated CommitTransaction calls to include accountMetadata parameter.

These function calls have been properly updated to match the new signature of CommitTransaction, which now accepts an additional accountMetadata parameter. The changes are consistent and properly implement the metadata handling optimization.

Also applies to: 68-68, 75-75, 82-82, 89-89, 96-96, 103-103


501-501: Updated CommitTransaction calls to include accountMetadata parameter.

These function calls have been properly updated to match the new signature of CommitTransaction. Passing nil for the metadata parameter is appropriate as the implementation handles this case by initializing an empty map internally.

Also applies to: 507-507, 513-513, 519-519, 525-525, 531-531, 537-537

internal/controller/ledger/store.go (1)

37-37: Interface change to pass account metadata directly to CommitTransaction

The Store interface has been updated to accept account metadata as a parameter to CommitTransaction. This allows for direct association of metadata with accounts during transaction processing, eliminating the need for a separate metadata update operation.

internal/controller/ledger/controller_default_test.go (2)

54-56: Updated test to accommodate new parameter

The test has been properly updated to include the new accountMetadata parameter (passed as nil) in the CommitTransaction call to match the interface change.


104-105: Updated test to accommodate new parameter

Similar to the previous test case, this test has been properly updated to include the new accountMetadata parameter in the CommitTransaction call.

internal/storage/ledger/transactions.go (2)

37-40: Enhanced method signature with account metadata

The CommitTransaction implementation now accepts account metadata directly, with appropriate nil-check initialization. This allows for consistent handling of account metadata during transaction processing, eliminating a separate update operation later.


57-57: Optimization: Directly use provided account metadata

This is the key optimization in the PR. Instead of creating new metadata objects and requiring a separate SQL UPDATE later, the implementation now directly uses the metadata provided in the accountMetadata parameter. This eliminates one SQL UPDATE query in cases where metadata is provided.

internal/controller/ledger/store_generated_test.go (2)

107-110: Updated mock implementation with new parameter

The generated mock implementation has been properly updated to include the new accountMetadata parameter in the CommitTransaction method.


115-118: Updated mock recorder with new parameter

The mock recorder has been properly updated to handle the new parameter in CommitTransaction calls during tests.

internal/storage/ledger/transactions_test.go (1)

43-43: Adaptation of test functions to updated method signature.

All calls to CommitTransaction have been consistently updated to include the new nil parameter for account metadata, properly matching the updated method signature in the implementation.

Also applies to: 52-52, 112-112, 133-133, 141-141, 192-192, 238-238, 260-260, 286-286, 330-335, 369-374, 432-432, 499-499, 506-506, 513-513, 520-520, 559-559, 685-685, 694-694, 703-703, 711-711, 735-735

internal/controller/ledger/controller_default.go (4)

201-202: Optimized account metadata handling.

Now passing payload.AccountMetadata directly to the CommitTransaction method, eliminating the need for a separate SQL update query for account metadata after transaction commit.


215-217: Proper null handling for reverted transactions.

Appropriately passing nil for account metadata when committing a revert transaction, as no new metadata needs to be updated in this scenario.


327-330: Streamlined transaction commit with metadata.

Now passing result.AccountMetadata directly to CommitTransaction, which eliminates the need for separate metadata update logic that was previously executed after the transaction commit.


396-399: Consistent null handling in revertTransaction method.

Similar to the implementation in importLog, appropriately passing nil for account metadata when committing a reversed transaction.

✨ Finishing Touches
  • 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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 using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@codecov
Copy link

codecov bot commented Mar 20, 2025

Codecov Report

Attention: Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.

Project coverage is 82.11%. Comparing base (fd576e1) to head (a8d8b2f).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
internal/controller/ledger/controller_default.go 75.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #793      +/-   ##
==========================================
+ Coverage   82.04%   82.11%   +0.06%     
==========================================
  Files         137      137              
  Lines        7448     7440       -8     
==========================================
- Hits         6111     6109       -2     
+ Misses       1027     1023       -4     
+ Partials      310      308       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@gfyrag gfyrag added this pull request to the merge queue Mar 20, 2025
Merged via the queue into main with commit f44ed96 Mar 20, 2025
10 checks passed
@gfyrag gfyrag deleted the feat/reduce-database-requests-with-metadata branch March 20, 2025 10:32
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.

3 participants