Gettotalsupply #1447
Gettotalsupply #1447
Conversation
WalkthroughThe recent updates in Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant RPC
participant misc.cpp
User->>RPC: Request total supply
RPC->>misc.cpp: Call gettotalsupply()
misc.cpp-->RPC: Return total supply including attack-related amounts
RPC-->User: Send total supply
User->>RPC: Request Zerocoin pool balance
RPC->>misc.cpp: Call getzerocoinpoolbalance()
misc.cpp-->RPC: Return Zerocoin pool balance
RPC-->User: Send Zerocoin pool balance
User->>RPC: Request CVE-2018-17144 forge amount
RPC->>misc.cpp: Call getCVE17144amount()
misc.cpp-->RPC: Return CVE-2018-17144 forge amount
RPC-->User: Send CVE-2018-17144 forge amount
Poem
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? 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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/rpc/misc.cpp (1 hunks)
Additional comments not posted (1)
src/rpc/misc.cpp (1)
1585-1587: Validate the accuracy of the hardcoded values for attack adjustments.Hardcoded values, especially those related to financial calculations, should be accurately sourced and verified. Consider adding a verification step or documentation to confirm these values.
|
Only the Zerocoin supply figure is confirmed while the Lelantus figures does not account for funds that we managed to blacklist during the attack. |
| total += 49839700000000; // The actual amount of coins forged during the Zerocoin attacks (the negative balance after the pool closed), you can verify the number by calling getzerocoinpoolbalance rpc | ||
| total += 3131972000000; // The remaining amount of forged coins during CVE-2018-17144 attacks, after subtracting locked coins and burnt Coins sent to unrecoverable address https://explorer.firo.org/tx/0b53178c1b22bae4c04ef943ee6d6d30f2483327fe9beb54952951592e8ce368 | ||
|
|
There was a problem hiding this comment.
Clarify the hardcoded values in the gettotalsupply function.
The function now includes hardcoded values to adjust for historical discrepancies due to attacks. It's crucial to document the source and calculation method of these values for future reference and auditability.
| UniValue getCVE17144amount(const JSONRPCRequest& request) | ||
| { | ||
| if (request.fHelp || request.params.size() != 0) | ||
| throw std::runtime_error( | ||
| "getCVE17144amount\n" | ||
| "\nReturns the total amount of forged coins during CVE-2018-17144 attacks.\n" | ||
| "\nArguments: none\n" | ||
| "\nResult:\n" | ||
| "{\n" | ||
| " \"total\" (string) The total balance\n" | ||
| "}\n" | ||
| "\nExamples:\n" | ||
| + HelpExampleCli("getCVE17144amount", "") | ||
| + HelpExampleRpc("getCVE17144amount", "") | ||
| ); | ||
| // as the attack happened at block 293526, | ||
| // get the block | ||
| CBlockIndex *mintBlock = chainActive[293526]; | ||
| CBlock block; | ||
| if (!ReadBlockFromDisk(block, mintBlock, ::Params().GetConsensus())) { | ||
| throw std::runtime_error(std::string("can't read block from disk, ")); | ||
| } | ||
| CAmount amount = 0; | ||
| for (CTransactionRef tx : block.vtx) { | ||
| std::set<COutPoint> vInOutPoints; | ||
| if (!tx->IsCoinBase() && !tx->HasNoRegularInputs()) { | ||
| std::set<COutPoint> vInOutPoints; | ||
| for (const auto& txin : tx->vin) | ||
| { | ||
| if (!vInOutPoints.insert(txin.prevout).second) { | ||
| CTransactionRef tx; | ||
| uint256 hashBlock; | ||
| if (!GetTransaction(txin.prevout.hash, tx, Params().GetConsensus(), hashBlock, true)) { | ||
| continue; | ||
| } | ||
| amount += tx->vout[txin.prevout.n].nValue; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| return amount; |
There was a problem hiding this comment.
Check calculation logic in getCVE17144amount.
The function calculates the total forged coins during the CVE-2018-17144 attacks. It's crucial to ensure that the block data is read correctly and that the transaction inputs are uniquely counted. The current implementation might double-count inputs due to the redeclaration of vInOutPoints inside the loop.
- std::set<COutPoint> vInOutPoints;
+ vInOutPoints.clear();Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| UniValue getCVE17144amount(const JSONRPCRequest& request) | |
| { | |
| if (request.fHelp || request.params.size() != 0) | |
| throw std::runtime_error( | |
| "getCVE17144amount\n" | |
| "\nReturns the total amount of forged coins during CVE-2018-17144 attacks.\n" | |
| "\nArguments: none\n" | |
| "\nResult:\n" | |
| "{\n" | |
| " \"total\" (string) The total balance\n" | |
| "}\n" | |
| "\nExamples:\n" | |
| + HelpExampleCli("getCVE17144amount", "") | |
| + HelpExampleRpc("getCVE17144amount", "") | |
| ); | |
| // as the attack happened at block 293526, | |
| // get the block | |
| CBlockIndex *mintBlock = chainActive[293526]; | |
| CBlock block; | |
| if (!ReadBlockFromDisk(block, mintBlock, ::Params().GetConsensus())) { | |
| throw std::runtime_error(std::string("can't read block from disk, ")); | |
| } | |
| CAmount amount = 0; | |
| for (CTransactionRef tx : block.vtx) { | |
| std::set<COutPoint> vInOutPoints; | |
| if (!tx->IsCoinBase() && !tx->HasNoRegularInputs()) { | |
| std::set<COutPoint> vInOutPoints; | |
| for (const auto& txin : tx->vin) | |
| { | |
| if (!vInOutPoints.insert(txin.prevout).second) { | |
| CTransactionRef tx; | |
| uint256 hashBlock; | |
| if (!GetTransaction(txin.prevout.hash, tx, Params().GetConsensus(), hashBlock, true)) { | |
| continue; | |
| } | |
| amount += tx->vout[txin.prevout.n].nValue; | |
| } | |
| } | |
| } | |
| } | |
| return amount; | |
| for (CTransactionRef tx : block.vtx) { | |
| std::set<COutPoint> vInOutPoints; | |
| if (!tx->IsCoinBase() && !tx->HasNoRegularInputs()) { | |
| vInOutPoints.clear(); | |
| for (const auto& txin : tx->vin) | |
| { | |
| if (!vInOutPoints.insert(txin.prevout).second) { | |
| CTransactionRef tx; | |
| uint256 hashBlock; | |
| if (!GetTransaction(txin.prevout.hash, tx, Params().GetConsensus(), hashBlock, true)) { | |
| continue; | |
| } | |
| amount += tx->vout[txin.prevout.n].nValue; | |
| } | |
| } | |
| } | |
| } |
| UniValue getzerocoinpoolbalance(const JSONRPCRequest& request) | ||
| { | ||
| if (request.fHelp || request.params.size() != 0) | ||
| throw std::runtime_error( | ||
| "getzerocoinpoolbalance\n" | ||
| "\nReturns the total coin amount, which remains after zerocoin pool closed.\n" | ||
| "\nArguments: none\n" | ||
| "\nResult:\n" | ||
| "{\n" | ||
| " \"total\" (string) The total balance\n" | ||
| "}\n" | ||
| "\nExamples:\n" | ||
| + HelpExampleCli("getzerocoinpoolbalance", "") | ||
| + HelpExampleRpc("getzerocoinpoolbalance", "") | ||
| ); | ||
|
|
||
| CAmount nTotalAmount = 0; | ||
|
|
||
| // Iterate over all mints | ||
| std::vector<std::pair<CAddressIndexKey, CAmount> > addressIndex; | ||
| if (GetAddressIndex(uint160(), AddressType::zerocoinMint, addressIndex)) { | ||
| for (std::vector<std::pair<CAddressIndexKey, CAmount> >::const_iterator it=addressIndex.begin(); it!=addressIndex.end(); it++) { | ||
| nTotalAmount += it->second; | ||
| } | ||
| } | ||
| addressIndex.clear(); | ||
|
|
||
| // Iterate over all spends | ||
| if (GetAddressIndex(uint160(), AddressType::zerocoinSpend, addressIndex)) { | ||
| for (std::vector < std::pair < CAddressIndexKey, CAmount > > ::const_iterator it = addressIndex.begin(); | ||
| it != addressIndex.end(); it++) { | ||
| nTotalAmount += it->second; | ||
| } | ||
| } | ||
|
|
||
| return UniValue(nTotalAmount); | ||
| } |
There was a problem hiding this comment.
Tip
Codebase Verification
The GetAddressIndex function includes basic error handling to check if the address index is enabled and to handle read failures. However, the getzerocoinpoolbalance function does not include error handling for cases where GetAddressIndex might fail.
Consider adding error handling for GetAddressIndex calls in getzerocoinpoolbalance.
- File:
src/rpc/misc.cpp - Lines: 1594-1630
Analysis chain
Review the implementation of getzerocoinpoolbalance.
This function calculates the remaining balance of the Zerocoin pool. It's important to ensure that the address index data used for calculations is accurate and up-to-date. Additionally, consider adding error handling for cases where GetAddressIndex might fail.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that GetAddressIndex is retrieving accurate data.
# Test: Check if GetAddressIndex returns expected results. Expect: Valid and accurate data.
rg --type cpp "GetAddressIndex" src/
Length of output: 972
Script:
#!/bin/bash
# Retrieve the implementation of GetAddressIndex from src/validation.cpp and src/validation.h
ast-grep --lang cpp --pattern 'bool GetAddressIndex($_, $_, $_)'
Length of output: 66
Script:
#!/bin/bash
# Use rg to find the exact location of GetAddressIndex in src/validation.cpp
rg --type cpp "GetAddressIndex" src/validation.cpp
# Display the implementation of GetAddressIndex from src/validation.cpp
rg --type cpp -A 20 "bool GetAddressIndex" src/validation.cpp
Length of output: 966
|
Putting this on KIV until Lelantus window is closed. |
No description provided.