Build fix #1553
Merged
Build fix #1553
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughThis update modifies Spark name-related RPC commands and consensus parameters. The Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant RPC_Server
participant Chain
participant SparkNameManager
User->>RPC_Server: getsparknamedata(sparkname)
RPC_Server->>Chain: Lock chain state
RPC_Server->>SparkNameManager: Query sparkname
SparkNameManager-->>RPC_Server: Return address, validUntil, additionalInfo
RPC_Server-->>User: Return JSON object {address, validUntil, additionalInfo}
User->>RPC_Server: getsparknametxdetails(txhash)
RPC_Server->>Chain: Lock chain state
RPC_Server->>Chain: Retrieve transaction by txhash
alt Valid Spark Name TX
RPC_Server->>SparkNameManager: Extract name and address
RPC_Server-->>User: Return JSON object {sparkname, sparkaddress}
else Error
RPC_Server-->>User: Error (unknown/invalid tx)
end
User->>RPC_Server: getsparknames(fOnlyOwn)
RPC_Server->>Chain: Lock chain state
RPC_Server->>SparkNameManager: Get all spark names
alt fOnlyOwn == true
RPC_Server->>SparkNameManager: Filter to wallet-owned names
end
SparkNameManager-->>RPC_Server: List of names and addresses
RPC_Server-->>User: Return array of {sparkname, sparkaddress}
Possibly related PRs
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ Finishing Touches
🪧 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/chainparams.cpp(1 hunks)src/firo_params.h(1 hunks)src/rpc/blockchain.cpp(3 hunks)src/wallet/rpcwallet.cpp(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/wallet/rpcwallet.cpp (2)
src/rpc/server.h (2)
HelpExampleCli(201-201)HelpExampleRpc(202-202)src/rpc/server.cpp (4)
HelpExampleCli(591-594)HelpExampleCli(591-591)HelpExampleRpc(596-600)HelpExampleRpc(596-596)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Create Source Distribution
- GitHub Check: Create Source Distribution
🔇 Additional comments (6)
src/chainparams.cpp (1)
419-419: Fix the usage of ZC_SIGMA_END_BLOCK constantThe assignment uses the
ZC_SIGMA_END_BLOCKconstant, which was defined with a trailing semicolon in firo_params.h. This will result in a double semicolon in this line, which is syntactically valid but not good practice.Make sure to remove the trailing semicolon from the
ZC_SIGMA_END_BLOCKdefinition in firo_params.h to ensure proper behavior.src/rpc/blockchain.cpp (4)
220-231: Improved RPC documentation and interface for getsparknamedataThe function signature has been updated to require exactly one parameter and the documentation now clearly indicates the function returns a JSON object with named fields instead of an array.
250-261: Enhanced return format for getsparknamedataThe return format has been improved to use a proper JSON object with named fields instead of a positional array, making the API more robust and self-documenting.
264-312: New RPC method getsparknametxdetailsAdded a new RPC method to retrieve spark name and address from a transaction hash. The implementation properly:
- Checks if Spark is activated
- Retrieves the transaction by hash
- Validates the transaction as a spark name transaction
- Returns the appropriate data or error messages
1797-1798: Updated RPC command registrationThe command registration has been properly updated to include the parameter name for
getsparknamedataand to register the newgetsparknametxdetailsmethod.src/wallet/rpcwallet.cpp (1)
3952-3999: LGTM! Well-implemented RPC rename with clear documentation.The renaming of
getsparknamestogetownsparknamesis well implemented with clear documentation and proper error handling. The function maintains its core functionality while adding a useful parameter to filter results by ownership.
psolstice
requested changes
Apr 17, 2025
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/rpc/blockchain.cpp(3 hunks)src/wallet/rpcwallet.cpp(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/wallet/rpcwallet.cpp
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Create Source Distribution
- GitHub Check: Create Source Distribution
🔇 Additional comments (2)
src/rpc/blockchain.cpp (2)
180-223: Newgetsparknamedataimplementation looks goodThe function has been properly implemented to take a spark name as a required parameter and return structured data about the spark name. It correctly checks for Spark activation before proceeding, ensuring backward compatibility with nodes that haven't activated this feature yet.
1758-1759: RPC commands registration updated correctlyThe RPC command registration table now includes the renamed
getsparknamedatamethod and the newgetsparknametxdetailsmethod, both with proper parameter specifications.
Comment on lines
+226
to
274
| UniValue getsparknametxdetails(const JSONRPCRequest &request) | ||
| { | ||
| if (request.fHelp || request.params.size() > 1) { | ||
| if (request.fHelp || request.params.size() != 1) { | ||
| throw std::runtime_error( | ||
| "getsparknamedata ( sparkname )\n" | ||
| "\nReturns info about spark name.\n" | ||
| "getsparknametxdetails (txhash)\n" | ||
| "\nReturns spark address and spark name associated with tx hash.\n" | ||
| "\nArguments:\n" | ||
| "Spark name (string)\n" | ||
| "1. txhash\n" | ||
| "\nResult:\n" | ||
| "[\n" | ||
| "1. Address (string)\n" | ||
| "2. Block height until this spark name is valid (int)\n" | ||
| "3. Additional info (string)\n" | ||
| " \"Name (string)\n" | ||
| " \"Address (string)\"\n" | ||
| " ...\n" | ||
| "]\n" | ||
| "\nExamples:\n" | ||
| + HelpExampleCli("getsparknamedata", "sparkname") | ||
| + HelpExampleRpc("getsparknamedata", "sparkname") | ||
| + HelpExampleCli("getsparknametxdetails", "txhash") | ||
| + HelpExampleRpc("getsparknametxdetails", "txhash") | ||
| ); | ||
| } | ||
|
|
||
| LOCK(cs_main); | ||
|
|
||
| if (!spark::IsSparkAllowed()) { | ||
| throw JSONRPCError(RPC_WALLET_ERROR, "Spark is not activated yet"); | ||
| } | ||
|
|
||
| std::string sparkName = request.params[0].get_str(); | ||
| CSparkNameManager *sparkNameManager = CSparkNameManager::GetInstance(); | ||
| std::string strTxId = request.params[0].get_str(); | ||
| uint256 txid = uint256S(strTxId); | ||
|
|
||
| std::string SparkAddr; | ||
| sparkNameManager->GetSparkAddress(sparkName, SparkAddr); | ||
|
|
||
| UniValue result(UniValue::VARR); | ||
| unsigned char network = spark::GetNetworkType(); | ||
| CTransactionRef txRef; | ||
| uint256 hashBlock; | ||
| if(!GetTransaction(txid, txRef, Params().GetConsensus(), hashBlock, true)) | ||
| throw JSONRPCError(RPC_TRANSACTION_ERROR, "Unknown transaction."); | ||
|
|
||
| result.push_back(SparkAddr); | ||
| CSparkNameTxData sparkNameData; | ||
| CValidationState state; | ||
| CSparkNameManager *sparkNameManager = CSparkNameManager::GetInstance(); | ||
|
|
||
| uint64_t nameBlockHeight = sparkNameManager->GetSparkNameBlockHeight(sparkName); | ||
| result.push_back(nameBlockHeight); | ||
| const CTransaction& tx = *txRef; | ||
| if (!sparkNameManager->CheckSparkNameTx(tx, chainActive.Height(), state, &sparkNameData)) | ||
| throw JSONRPCError(RPC_TRANSACTION_ERROR, "Invalid spark tx hash"); | ||
|
|
||
| std::string sparkNameData = sparkNameManager->GetSparkNameAdditionalData(sparkName); | ||
| result.push_back(sparkNameData); | ||
| if (sparkNameData.name.empty() && sparkNameData.sparkAddress.empty()) | ||
| throw JSONRPCError(RPC_TRANSACTION_ERROR, "Invalid spark name tx hash"); | ||
|
|
||
| UniValue result(UniValue::VOBJ); | ||
| result.push_back(Pair("name", sparkNameData.name)); | ||
| result.push_back(Pair("address", sparkNameData.sparkAddress)); | ||
| return result; | ||
| } |
There was a problem hiding this comment.
Inconsistency between documentation and implementation in getsparknametxdetails
The implementation properly retrieves and validates a spark name transaction, but there's a discrepancy between the help text and the actual return value:
- The help text in lines 235-239 describes the result as a JSON array
[ "Name (string)", "Address (string)", ... ] - The actual implementation in lines 270-273 returns a JSON object
{ "name": "...", "address": "..." }
Update the help text to match the actual implementation:
"\nResult:\n"
- "[\n"
- " \"Name (string)\n"
- " \"Address (string)\"\n"
- " ...\n"
- "]\n"
+ "{\n"
+ " \"name\": \"...\", (string) The spark name\n"
+ " \"address\": \"...\" (string) The spark address\n"
+ "}\n"
psolstice
approved these changes
Apr 18, 2025
levoncrypto
approved these changes
Apr 18, 2025
aleflm
added a commit
that referenced
this pull request
Apr 23, 2025
Rebase includes: * UI improvements (#1475) * Lelantus UI stripped * More improvements * Fixed UI hang issue during batch verification * QT hanging improvements * QT hanging fix * Review commits applied * coderabbit comments applied * more coderabbitai comments resolved --------- Co-authored-by: firstcryptoman <firstcryptoman@gmail.com> * Spark names (#1532) * Initial spark name architecture * Spark address ownership proofs implemented. * Missing files added * Check the ownership proof for spark names: initial implementation * Fixes to the core part of spark names * Added additional field (core) * Consensus parameters for spark names * Fixed mempool bug * Fixes in spark name conflict resolution * RPCs for spark names * Additional API for spark names tx creation * Changed way of checking spark name tx * Wallet API for spark name transaction creation * API changes for spark name tx creation * Added registersparkname RPC call * Spark activation check for RPC * Make spark names case-insensitive * Spark name RPC fixes * Faster lookup for spark name by spark address * Fixes for spark name/address lookup * Improvements for duplicated address detection * Fixes for spark name state * Block index entries for spark names * Make dot (.) a legit symbol in spark name * Spark name block number for testnet * Fixed restoration of old spark name data if block is disconnected * API for quick check of spark name transaction validity before the creation * added isAddressMine function * Check if the address belongs to the wallet before creating spark name transaction * Fixed fee calculation for spark name * Fix for spark names RPC * Added ability to spend to spark names in "sparkspend" RPC * UI fixes * Additional validations * Fix for crash with spend to unregistered name * Fixed fee value when registering spark name for more than one year * Spark name UI improvements * UI modifications for sending to spark name * Address book fixes related to spark names * Fixed period of validity when creating spark name through GUI * Extended spark name error info for RPC * Fixed crash on non-HD wallet * Request wallet password for spark name creation * Fixed fee calculation for the spark name tx * Fixed detection of previously used spark address for a spark name * Unit test for spark names * Additional unit tests * Fixes #1533 * getsparknamedata RPC now returns JSON object * Added "My own spark names" to the dropdown list in address book * Added an option of displaying only own spark names for RPC. Closes #1535 * Set HF block for spark names * Fixed a check for spark name block start * Added tests for correctly respecting HF block number * Check if we're over HF before spark name transaction creation * new rpc for spark name (#1552) * Fixed spark name tests * Changed HF date * Change of HF block number --------- Co-authored-by: levonpetrosyan93 <petrosyan.levon93@gmail.com> Co-authored-by: levoncrypto <levoncrypto1994@gmail.com> Co-authored-by: levoncrypto <95240473+levoncrypto@users.noreply.github.com> Co-authored-by: levonpetrosyan93 <45027856+levonpetrosyan93@users.noreply.github.com> * Export View Keys (#1543) * Add an RPC command to export the Spark view key. * Show Spark View Key in Qt. * Sigma pool closed, Extra payload extended (#1477) * Change of emission rules * Fixes for testnet * Cleaning up code and tests * Workaround for current devnet bugs * Workaround for testnet * Devnet parameter tweak * Sigma pool closed * Extra payload size limit increased * Changed HF block for testnet * Initial spark name architecture * Spark address ownership proofs implemented. * Missing files added * Check the ownership proof for spark names: initial implementation * Fixes to the core part of spark names * Added additional field (core) * Consensus parameters for spark names * Fixed mempool bug * Fixes in spark name conflict resolution * RPCs for spark names * Additional API for spark names tx creation * Changed way of checking spark name tx * Wallet API for spark name transaction creation * API changes for spark name tx creation * Added registersparkname RPC call * Spark activation check for RPC * Make spark names case-insensitive * Spark name RPC fixes * Faster lookup for spark name by spark address * Fixes for spark name/address lookup * Improvements for duplicated address detection * Fixes for spark name state * Block index entries for spark names * Make dot (.) a legit symbol in spark name * Spark name block number for testnet * Fixed restoration of old spark name data if block is disconnected * API for quick check of spark name transaction validity before the creation * added isAddressMine function * Check if the address belongs to the wallet before creating spark name transaction * Fixed fee calculation for spark name * Fix for spark names RPC * Added ability to spend to spark names in "sparkspend" RPC * UI fixes * Additional validations * Fix for crash with spend to unregistered name * Fixed fee value when registering spark name for more than one year * Spark name UI improvements * UI modifications for sending to spark name * Address book fixes related to spark names * Fixed period of validity when creating spark name through GUI * Extended spark name error info for RPC * Fixed crash on non-HD wallet * Request wallet password for spark name creation * Fixed fee calculation for the spark name tx * Fixed detection of previously used spark address for a spark name * Unit test for spark names * Additional unit tests * Fixes #1533 * Testnet HF block set * Mainnet HF block set --------- Co-authored-by: Peter Shugalev <peter@shugalev.com> Co-authored-by: levoncrypto <levoncrypto1994@gmail.com> * Build fix (#1553) * Build fix * coderabbitai comment resolved * Duplicated rpc removed * Bump version to v0.14.14.1 Spark Names (#1550) * secp256k1: CMake build system added * cmake: add cmake folder * bench: Add initial cmake support * Add initial main CMakeLists.txt * doc: add initial cmake support * univalue: add initial cmake support * zmq: add initial cmake support * crypto: add initial cmake support * wallet: add initial cmake support * src: initial add of src/CMakeLists.txt * toolchain.cmake.in: Adding toolchain.cmake.in support * crypto: add support for CMake function check. * bitcoin-cli: add CMake compilation. * firo-tx: add CMake compilation. Improve miscellaneous CMake files. * firod: add CMake compilation support. * Improved misc CMake compilation support. * Add bitcoin-config.h generation support. * gmp: fix gmp link on OS X * build: generate Linux installable package. * CMake: add tests (test_bitcoin) compilation support. * Qt: CMake GUI compilation support (Ubuntu) * src/qt: Add macOS support for GUI with CMake compilation. * depends: fix gmp compilation issue with mingw * build: Add MingW support for CMake build system * src: add backtrace compilation support macOS * src: add backtrace compilation support Linux and MinGW-w64 * CMake: apply CodeRabbitAI suggestions. * CMake: Add CI tasks for CMake build. * Remove static from functions to fix debug linking In release builds, these functions are inlined, avoiding linkage issues. Debug builds (without inlining) caused undefined references in dependent libraries due to static visibility. Removing static resolves this while maintaining intended behavior in optimized builds. * Add removed testcases and make BUILD_GUI=ON default option * Added documentation to readme.md and fixed a lelantus test issue * Fixed documentation, set ENABLE_WALLET=ON by default Remove outdated old sigma testcase * Rebase to Wed Apr 23 11:39:34 AM BST 2025 --------- Co-authored-by: levonpetrosyan93 <45027856+levonpetrosyan93@users.noreply.github.com> Co-authored-by: firstcryptoman <firstcryptoman@gmail.com> Co-authored-by: psolstice <peter@shugalev.com> Co-authored-by: levonpetrosyan93 <petrosyan.levon93@gmail.com> Co-authored-by: levoncrypto <levoncrypto1994@gmail.com> Co-authored-by: levoncrypto <95240473+levoncrypto@users.noreply.github.com> Co-authored-by: cassandras-lies <203535133+cassandras-lies@users.noreply.github.com> Co-authored-by: justanwar <42809091+justanwar@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.