Skip to content

Commit 39407b8

Browse files
UdjinM6claude
andcommitted
refactor: address code review feedback
- Add null check for pindex->pprev in BlockDisconnected - Add address type validation in AddressIndex iterator loops - Add outputIndex >= 0 validation in getspentinfo RPC - Add EXCLUSIVE_LOCKS_REQUIRED(::cs_main) to MigrateOldIndexData - Use const reference for SpentIndex::GetSpentInfo key parameter - Move m_db to private section in AddressIndex - Add missing <memory> include in addressindex.h - Fix unused variable warning (use _ placeholder) - Fix docstring in feature_spentindex.py - Add .cpp patterns to non-backported.txt 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent d52a3cb commit 39407b8

File tree

9 files changed

+24
-11
lines changed

9 files changed

+24
-11
lines changed

src/index/addressindex.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,8 @@ bool AddressIndex::DB::ReadAddressIndex(const uint160& address_hash, const Addre
6060

6161
while (pcursor->Valid()) {
6262
std::pair<uint8_t, CAddressIndexKey> key;
63-
if (pcursor->GetKey(key) && key.first == DB_ADDRESSINDEX && key.second.m_address_bytes == address_hash) {
63+
if (pcursor->GetKey(key) && key.first == DB_ADDRESSINDEX &&
64+
key.second.m_address_type == type && key.second.m_address_bytes == address_hash) {
6465
if (end > 0 && key.second.m_block_height > end) {
6566
break;
6667
}
@@ -89,7 +90,8 @@ bool AddressIndex::DB::ReadAddressUnspentIndex(const uint160& address_hash, cons
8990

9091
while (pcursor->Valid()) {
9192
std::pair<uint8_t, CAddressUnspentKey> key;
92-
if (pcursor->GetKey(key) && key.first == DB_ADDRESSUNSPENTINDEX && key.second.m_address_bytes == address_hash) {
93+
if (pcursor->GetKey(key) && key.first == DB_ADDRESSUNSPENTINDEX &&
94+
key.second.m_address_type == type && key.second.m_address_bytes == address_hash) {
9395
CAddressUnspentValue value;
9496
if (pcursor->GetValue(value)) {
9597
entries.emplace_back(key.second, value);
@@ -116,7 +118,7 @@ bool AddressIndex::DB::EraseAddressIndex(const std::vector<CAddressIndexEntry>&
116118
{
117119
CDBBatch batch(*this);
118120

119-
for (const auto& [key, value] : entries) {
121+
for (const auto& [key, _] : entries) {
120122
batch.Erase(std::make_pair(DB_ADDRESSINDEX, key));
121123
}
122124

@@ -381,7 +383,7 @@ void AddressIndex::BlockDisconnected(const std::shared_ptr<const CBlock>& block,
381383
const CBlockIndex* best_block_index = CurrentIndex();
382384

383385
// Only rewind if we have this block indexed
384-
if (best_block_index && best_block_index->nHeight >= pindex->nHeight) {
386+
if (best_block_index && best_block_index->nHeight >= pindex->nHeight && pindex->pprev) {
385387
if (!Rewind(best_block_index, pindex->pprev)) {
386388
error("%s: Failed to rewind %s to previous block after disconnect",
387389
__func__, GetName());

src/index/addressindex.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include <index/addressindex_types.h>
99
#include <index/base.h>
1010

11+
#include <memory>
1112
#include <vector>
1213

1314
static constexpr bool DEFAULT_ADDRESSINDEX{false};
@@ -24,8 +25,11 @@ class AddressIndex final : public BaseIndex
2425
{
2526
protected:
2627
class DB;
28+
29+
private:
2730
const std::unique_ptr<DB> m_db;
2831

32+
protected:
2933
class DB : public BaseIndex::DB
3034
{
3135
public:

src/index/spentindex.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ void SpentIndex::BlockDisconnected(const std::shared_ptr<const CBlock>& block, c
152152
const CBlockIndex* best_block_index = CurrentIndex();
153153

154154
// Only rewind if we have this block indexed
155-
if (best_block_index && best_block_index->nHeight >= pindex->nHeight) {
155+
if (best_block_index && best_block_index->nHeight >= pindex->nHeight && pindex->pprev) {
156156
if (!Rewind(best_block_index, pindex->pprev)) {
157157
error("%s: Failed to rewind %s to previous block after disconnect",
158158
__func__, GetName());
@@ -162,7 +162,7 @@ void SpentIndex::BlockDisconnected(const std::shared_ptr<const CBlock>& block, c
162162

163163
BaseIndex::DB& SpentIndex::GetDB() const { return *m_db; }
164164

165-
bool SpentIndex::GetSpentInfo(CSpentIndexKey& key, CSpentIndexValue& value) const
165+
bool SpentIndex::GetSpentInfo(const CSpentIndexKey& key, CSpentIndexValue& value) const
166166
{
167167
if (!BlockUntilSyncedToCurrentChain()) {
168168
return false;

src/index/spentindex.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ class SpentIndex final : public BaseIndex
7373
virtual ~SpentIndex() override;
7474

7575
/// Retrieve spent information for a specific output
76-
bool GetSpentInfo(CSpentIndexKey& key, CSpentIndexValue& value) const;
76+
bool GetSpentInfo(const CSpentIndexKey& key, CSpentIndexValue& value) const;
7777
};
7878

7979
/// Global SpentIndex instance

src/index/timestampindex.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,8 @@ void TimestampIndex::BlockDisconnected(const std::shared_ptr<const CBlock>& bloc
9393
// to remove this block's data
9494
const CBlockIndex* best_block_index = CurrentIndex();
9595

96-
// Only rewind if we have this block indexed
97-
if (best_block_index && best_block_index->nHeight >= pindex->nHeight) {
96+
// Only rewind if we have this block indexed and it's not the genesis block
97+
if (best_block_index && best_block_index->nHeight >= pindex->nHeight && pindex->pprev) {
9898
if (!Rewind(best_block_index, pindex->pprev)) {
9999
error("%s: Failed to rewind %s to previous block after disconnect",
100100
__func__, GetName());

src/rpc/node.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -811,6 +811,10 @@ static RPCHelpMan getspentinfo()
811811
uint256 txid = ParseHashV(txidValue, "txid");
812812
int outputIndex = indexValue.getInt<int>();
813813

814+
if (outputIndex < 0) {
815+
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid index (must be non-negative)");
816+
}
817+
814818
if (!g_spentindex) {
815819
throw JSONRPCError(RPC_MISC_ERROR, "Spent index is not enabled. Start with -spentindex to enable.");
816820
}

src/txdb.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ class CBlockTreeDB : public CDBWrapper
9494
EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
9595

9696
/// Migrate old synchronous index data to new async index databases
97-
bool MigrateOldIndexData();
97+
bool MigrateOldIndexData() EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
9898
};
9999

100100
#endif // BITCOIN_TXDB_H

test/functional/feature_spentindex.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
55

66
#
7-
# Test addressindex generation and fetching
7+
# Test spentindex generation and fetching
88
#
99

1010
from decimal import Decimal

test/util/data/non-backported.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,11 @@ src/evo/*.cpp
1818
src/evo/*.h
1919
src/governance/*.cpp
2020
src/governance/*.h
21+
src/index/addres*.cpp
2122
src/index/addres*.h
23+
src/index/spent*.cpp
2224
src/index/spent*.h
25+
src/index/timestamp*.cpp
2326
src/index/timestamp*.h
2427
src/instantsend/*.cpp
2528
src/instantsend/*.h

0 commit comments

Comments
 (0)