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

New rpc calls for mobile #1449

Merged
merged 7 commits into from
Jun 18, 2024
Merged

New rpc calls for mobile #1449

merged 7 commits into from
Jun 18, 2024

Conversation

levonpetrosyan93
Copy link
Contributor

@levonpetrosyan93 levonpetrosyan93 commented Jun 9, 2024

Added new rpc calls,

  • getusedcoinstagstxhashes, this rpc returns all spent coin serials attached to the hash of spending transaction, previously we had getusedcoinstags , the new call allows to get also in which transaction is spent the coin, to do this we add field inside block index, and in spark state, when tx is being checked during block acceptance, we add the pair of ltag and tx hash into block index, and each time we construct the state, we get the data from the index. this fields are constructed in case -mobile is passed,
  • getmempooltxids gives the list of spark tx hashes inside mempool, this allow to get spark state not only for blockchian but also from mempool.
  • getmempooltxs returns spark metadata for each tx hash, existing in mempool, in case there is no data for particular tx, that means tx is removed from mempool, confirmed or removed for another reason.

Copy link

coderabbitai bot commented Jun 9, 2024

Walkthrough

The recent changes encompass various enhancements to the RPC methods and the handling of transaction hashes within the blockchain. Key updates include new RPC commands for retrieving mempool transaction IDs and tags, modifications to the CBlockIndex class to incorporate a ltagTxhash map, and updates to the CSparkState class to manage and retrieve transaction hashes. These improvements aim to extend functionality and provide more granularity in transaction data management.

Changes

File(s) Change Summary
src/chain.h Added ltagTxhash map to CBlockIndex, cleared in constructor and destructor, serialized based on a flag.
src/rpc/client.cpp Added new RPC method getmempooltxs.
src/rpc/misc.cpp Introduced new functions for retrieving used coin tags, mempool transaction IDs, and spark data.
src/rpc/server.cpp Added new RPC commands to retrieve transaction data.
src/rpc/server.h Added declarations for new RPC functions.
src/spark/state.cpp Enhanced CSparkState with methods for managing ltagTxhash and iterating over it conditionally.
src/spark/state.h Added methods to CSparkState for inserting and retrieving elements in ltagTxhash.
src/txdb.cpp Assigned ltagTxhash within LoadBlockIndexGuts.

Sequence Diagrams

AddLTagTxHash Interaction

sequenceDiagram
    participant CSparkState
    participant CBlockIndex
    participant CTransaction
    CSparkState->>CBlockIndex: AddLTagTxHash(lTagHash, txHash)
    CBlockIndex->>CSparkState: ltagTxhash[lTagHash] = txHash
    CTransaction-->>CSparkState: Trigger ltagTxhash update on transaction spend
Loading

getmempooltxs RPC Method

sequenceDiagram
    participant Client
    participant RPCServer
    participant Mempool
    Client->>RPCServer: getmempooltxs
    RPCServer->>Mempool: Retrieve transactions
    Mempool-->>RPCServer: Return transactions
    RPCServer-->>Client: Respond with transaction data
Loading

Poem

Amidst the chains and coded lore,
A rabbit brings updates to explore.
With mempool data and hashes anew,
Our blockchain grows with every cue.
Tags and IDs set to flow,
In code's vast field, our knowledge grows.
Hopping through changes, here and there,
The blockchain's story we all share. 🌐🐇✨


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?

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>.
    • 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 generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @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 as 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 resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration 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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between cf19353 and c76d862.

Files selected for processing (8)
  • src/chain.h (3 hunks)
  • src/rpc/client.cpp (1 hunks)
  • src/rpc/misc.cpp (3 hunks)
  • src/rpc/server.cpp (1 hunks)
  • src/rpc/server.h (1 hunks)
  • src/spark/state.cpp (5 hunks)
  • src/spark/state.h (4 hunks)
  • src/txdb.cpp (1 hunks)
Files skipped from review due to trivial changes (1)
  • src/rpc/client.cpp
Additional comments not posted (17)
src/rpc/server.h (3)

219-219: The declaration of getusedcoinstagstxhashes is correctly added.


221-221: The declaration of getmempooltxids is correctly added.


222-222: The declaration of getmempooltxs is correctly added.

src/spark/state.h (3)

29-29: The addition of ltagTxhash member in CSparkState is correctly implemented to store linking tag hashes mapped to transaction hashes.


172-172: The addition of AddLTagTxHash method in CSparkState is correctly implemented to handle the insertion of linking tag hashes.


218-218: The addition of GetSpendTxIds method in CSparkState is correctly implemented to retrieve linking tag hashes.

src/rpc/server.cpp (3)

345-345: The addition of getusedcoinstagstxhashes command in the mobile category is correctly implemented.


347-347: The addition of getmempooltxids command in the mobile category is correctly implemented.


348-348: The addition of getmempooltxs command in the mobile category is correctly implemented.

src/chain.h (2)

264-265: The addition of ltagTxhash to CBlockIndex is well-implemented. Ensure that the hash map is used securely and efficiently.


308-308: Clearing ltagTxhash in the SetNull method is a good practice to ensure that the object starts with a clean state.

src/txdb.cpp (1)

428-428: The assignment of ltagTxhash from diskindex to pindexNew ensures that the transaction hash map is correctly loaded into the block index. This is crucial for the new RPC functionalities introduced in the PR.

Verification successful

The ltagTxhash is correctly utilized in src/rpc/misc.cpp for mapping transaction IDs, which aligns with the intended functionality. The assignment in src/txdb.cpp ensures that this mapping is correctly loaded into the block index.

  • src/rpc/misc.cpp: std::unordered_map<uint256, uint256> ltagTxhash;
  • src/rpc/misc.cpp: ltagTxhash = sparkState->GetSpendTxIds();
  • src/rpc/misc.cpp: if (ltagTxhash.count(ltagHash) > 0)
  • src/rpc/misc.cpp: txid = ltagTxhash[ltagHash];
Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify that `ltagTxhash` is correctly utilized in the new RPC methods.

# Test: Search for the usage of `ltagTxhash` in the new RPC methods. Expect: Proper handling and no errors.
rg --type cpp $'ltagTxhash' src/rpc/

Length of output: 291

src/spark/state.cpp (5)

268-277: The handling of -mobile flag for conditional execution is consistent across the codebase.


1090-1094: The conditional logic for handling the -mobile flag during block addition is implemented correctly. This ensures that mobile-specific data is handled appropriately based on runtime configuration.


1341-1343: The method GetSpendTxIds provides a consistent interface to access ltagTxhash. This is a good practice as it encapsulates the data structure, allowing for easier maintenance and potential future changes.


1050-1052: The method AddLTagTxHash is added to handle the insertion of tag-hash pairs into ltagTxhash. Ensure that this method is used consistently wherever ltagTxhash is modified.

Verification successful

The search results indicate only one direct modification to ltagTxhash, which is within the AddLTagTxHash method itself. This confirms that the method is used consistently for modifying ltagTxhash.

  • src/spark/state.cpp: The direct modification found is within the AddLTagTxHash method.
Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify consistent usage of `AddLTagTxHash` method across the codebase.

# Test: Search for direct modifications to `ltagTxhash` that bypass `AddLTagTxHash`.
rg --type cpp $'ltagTxhash\[.*\] ='

Length of output: 91


702-704: The conditional insertion of ltagTxhash based on the -mobile flag is implemented correctly. However, ensure that the function primitives::GetLTagHash is robust and handles edge cases effectively.

src/rpc/misc.cpp Outdated Show resolved Hide resolved
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between c76d862 and 973eefe.

Files selected for processing (1)
  • src/rpc/misc.cpp (3 hunks)
Additional comments not posted (1)
src/rpc/misc.cpp (1)

1510-1588: The implementation of getmempooltxs appears to be correct, but ensure that the method is thoroughly tested, especially the parsing and handling of transaction data.

src/rpc/misc.cpp Outdated Show resolved Hide resolved
src/rpc/misc.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@psolstice psolstice left a comment

Choose a reason for hiding this comment

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

Could you please add the description of what's the problem and how it's fixed. I see the coderabbit's comments, but this is not enough to do the review. Especially please explain the reasons behind adding new members of CBlockIndex.

@levonpetrosyan93
Copy link
Contributor Author

@psolstice the description added.

@reubenyap reubenyap added this to the Release 0.14.13.3 milestone Jun 17, 2024
src/rpc/misc.cpp Outdated Show resolved Hide resolved
src/rpc/misc.cpp Outdated Show resolved Hide resolved
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 973eefe and e7f6931.

Files selected for processing (3)
  • src/rpc/misc.cpp (3 hunks)
  • src/rpc/server.cpp (1 hunks)
  • src/rpc/server.h (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • src/rpc/server.cpp
Additional comments not posted (4)
src/rpc/server.h (1)

221-221: Ensure consistency in naming and functionality for getmempoolsparktxids.

This function is supposed to return all spark transaction IDs currently in the mempool. Verify its implementation in src/rpc/misc.cpp to ensure it aligns with the naming conventions and functionality described.

Verification successful

The getmempoolsparktxids function in src/rpc/misc.cpp is correctly implemented to return all spark transaction IDs currently in the mempool. It checks for help requests or incorrect parameters, retrieves all transactions from the mempool, filters for spark transactions, and returns their IDs.

  • The function name getmempoolsparktxids accurately reflects its functionality.
Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Search for the implementation of `getmempoolsparktxids` in `src/rpc/misc.cpp`
fd "misc.cpp" src/rpc/ --exec cat {} | grep -A 3 -B 3 "getmempoolsparktxids"

Length of output: 904



Script:

#!/bin/bash
# Retrieve the full implementation of the `getmempoolsparktxids` function in `src/rpc/misc.cpp`
ast-grep --lang cpp --pattern 'UniValue getmempoolsparktxids(const JSONRPCRequest& request) { $$$ }' src/rpc/misc.cpp

Length of output: 119



Script:

#!/bin/bash
# Reattempt to retrieve the full implementation of the `getmempoolsparktxids` function in `src/rpc/misc.cpp`
rg -A 20 -B 5 'UniValue getmempoolsparktxids' src/rpc/misc.cpp

Length of output: 769

src/rpc/misc.cpp (3)

1407-1462: Exception handling for std::stol is correctly implemented, ensuring robust input validation.


1491-1508: Proper implementation of getmempoolsparktxids to retrieve Spark transaction IDs from the mempool.


1510-1588: Consider clarifying the RPC help description for getmempoolsparktxs to better explain the expected input and output data formats.
[ISSU

- "\nReturns spark data for each transaction.\n"
+ "\nReturns detailed Spark transaction data for each specified transaction ID.\n"

@@ -216,7 +216,10 @@ extern UniValue getlatestcoinid(const JSONRPCRequest& params);
extern UniValue getsparkanonymityset(const JSONRPCRequest& params);
extern UniValue getsparkmintmetadata(const JSONRPCRequest& params);
extern UniValue getusedcoinstags(const JSONRPCRequest& params);
extern UniValue getusedcoinstagstxhashes(const JSONRPCRequest& params);
Copy link

Choose a reason for hiding this comment

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

Add documentation for getusedcoinstagstxhashes.

The function getusedcoinstagstxhashes is introduced to extend the functionality of the existing getusedcoinstags by also providing the transaction hash in which the coin was spent. It's crucial to have proper documentation for new RPC methods to ensure they are used correctly and understood by other developers or API consumers.

src/rpc/server.h Show resolved Hide resolved
src/rpc/misc.cpp Outdated Show resolved Hide resolved
psolstice
psolstice previously approved these changes Jun 18, 2024
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between e7f6931 and f7415c7.

Files selected for processing (1)
  • src/rpc/misc.cpp (3 hunks)

src/rpc/misc.cpp Show resolved Hide resolved
src/rpc/misc.cpp Show resolved Hide resolved
src/rpc/misc.cpp Show resolved Hide resolved
@reubenyap reubenyap merged commit 290730d into master Jun 18, 2024
6 checks passed
@reubenyap reubenyap deleted the mempool_mobile branch June 18, 2024 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants