From 3ab038e7792f3b83c590bb215c4480786d98351e Mon Sep 17 00:00:00 2001 From: Lee *!* Clagett Date: Mon, 3 Jun 2024 19:41:44 -0400 Subject: [PATCH] Fix several bugs: (#119) * lws::account height update should only go up. * Webhook confirmations can start after first new block * Webhook confirmations could face a rescan --- src/db/account.cpp | 2 +- src/db/account.h | 2 +- src/db/storage.cpp | 66 +++++++++++++++++---------------- tests/unit/db/chain.test.cpp | 13 +++++++ tests/unit/db/webhook.test.cpp | 67 +++++++++++++++++++++++++++++++--- tests/unit/scanner.test.cpp | 2 +- 6 files changed, 113 insertions(+), 39 deletions(-) diff --git a/src/db/account.cpp b/src/db/account.cpp index 7752827..a6d504d 100644 --- a/src/db/account.cpp +++ b/src/db/account.cpp @@ -165,7 +165,7 @@ namespace lws void account::updated(db::block_id new_height) noexcept { - height_ = new_height; + height_ = std::max(height_, new_height); spends_.clear(); spends_.shrink_to_fit(); outputs_.clear(); diff --git a/src/db/account.h b/src/db/account.h index c044081..1904744 100644 --- a/src/db/account.h +++ b/src/db/account.h @@ -88,7 +88,7 @@ namespace lws //! \return A copy of `this`. account clone() const; - //! \return A copy of `this` with a new height and `outputs().empty()`. + //! \post `height() == max(new_height, height())`, `outputs().empty()`, and `spends.empty()`. void updated(db::block_id new_height) noexcept; //! \return Unique ID from the account database, possibly `db::account_id::kInvalid`. diff --git a/src/db/storage.cpp b/src/db/storage.cpp index 6a30a9a..2be17d6 100644 --- a/src/db/storage.cpp +++ b/src/db/storage.cpp @@ -2537,38 +2537,42 @@ namespace db const webhook_event event = MONERO_UNWRAP(events_by_account_id.get_value(value)); - MDB_val rkey = lmdb::to_val(hook_key); - MDB_val rvalue = lmdb::to_val(event.link_webhook); - MONERO_LMDB_CHECK(mdb_cursor_get(&webhooks_cur, &rkey, &rvalue, MDB_GET_BOTH)); - - MDB_val okey = lmdb::to_val(user); - MDB_val ovalue = lmdb::to_val(event.link); - MONERO_LMDB_CHECK(mdb_cursor_get(&outputs_cur, &okey, &ovalue, MDB_GET_BOTH)); - - events.push_back( - webhook_tx_confirmation{ - MONERO_UNWRAP(webhooks.get_key(rkey)), - MONERO_UNWRAP(webhooks.get_value(rvalue)), - MONERO_UNWRAP(outputs.get_value(ovalue)) - } - ); - - const std::uint32_t requested_confirmations = - events.back().value.second.confirmations; - - events.back().value.second.confirmations = - lmdb::to_native(begin) - lmdb::to_native(event.link.tx.height) + 1; - - // copy next blocks from first - for (const auto block_num : boost::counting_range(lmdb::to_native(begin) + 1, lmdb::to_native(end))) + const block_id this_begin = std::max(begin, event.link.tx.height); + if (this_begin < end) { + MDB_val rkey = lmdb::to_val(hook_key); + MDB_val rvalue = lmdb::to_val(event.link_webhook); + MONERO_LMDB_CHECK(mdb_cursor_get(&webhooks_cur, &rkey, &rvalue, MDB_GET_BOTH)); + + MDB_val okey = lmdb::to_val(user); + MDB_val ovalue = lmdb::to_val(event.link); + MONERO_LMDB_CHECK(mdb_cursor_get(&outputs_cur, &okey, &ovalue, MDB_GET_BOTH)); + + events.push_back( + webhook_tx_confirmation{ + MONERO_UNWRAP(webhooks.get_key(rkey)), + MONERO_UNWRAP(webhooks.get_value(rvalue)), + MONERO_UNWRAP(outputs.get_value(ovalue)) + } + ); + + const std::uint32_t requested_confirmations = + events.back().value.second.confirmations; + + events.back().value.second.confirmations = + lmdb::to_native(this_begin) - lmdb::to_native(event.link.tx.height) + 1; + + // copy next blocks from first + for (const auto block_num : boost::counting_range(lmdb::to_native(this_begin) + 1, lmdb::to_native(end))) + { + if (requested_confirmations <= events.back().value.second.confirmations) + break; + events.push_back(events.back()); + ++(events.back().value.second.confirmations); + } if (requested_confirmations <= events.back().value.second.confirmations) - break; - events.push_back(events.back()); - ++(events.back().value.second.confirmations); - } - if (requested_confirmations <= events.back().value.second.confirmations) - MONERO_LMDB_CHECK(mdb_cursor_del(&events_cur, 0)); + MONERO_LMDB_CHECK(mdb_cursor_del(&events_cur, 0)); + } err = mdb_cursor_get(&events_cur, &key, &value, MDB_NEXT_DUP); } return success(); @@ -2777,7 +2781,7 @@ namespace db const block_id existing_height = existing->scan_height; - existing->scan_height = block_id(last_update); + existing->scan_height = std::max(existing_height, block_id(last_update)); value = lmdb::to_val(*existing); MONERO_LMDB_CHECK(mdb_cursor_put(accounts_cur.get(), &key, &value, MDB_CURRENT)); diff --git a/tests/unit/db/chain.test.cpp b/tests/unit/db/chain.test.cpp index 4fbd88e..28df82c 100644 --- a/tests/unit/db/chain.test.cpp +++ b/tests/unit/db/chain.test.cpp @@ -130,6 +130,19 @@ LWS_CASE("db::storage::sync_chain") const auto sync_result = db.sync_chain(lws::db::block_id(point->first), fchain); EXPECT(sync_result == lws::error::bad_blockchain); } + + SECTION("Old Blocks dont rollback height") + { + lws::account accounts[1] {lws::account{get_account(), {}, {}}}; + const auto old_block = lws::db::block_id(lmdb::to_native(last_block.id) - 5); + const auto new_block = lws::db::block_id(lmdb::to_native(last_block.id) + 4); + EXPECT(accounts[0].scan_height() == new_block); + EXPECT(db.update(old_block, chain, accounts, nullptr)); + EXPECT(get_account().scan_height == new_block); + + accounts[0].updated(old_block); + EXPECT(accounts[0].scan_height() == new_block); + } } } diff --git a/tests/unit/db/webhook.test.cpp b/tests/unit/db/webhook.test.cpp index dc90f04..fe1af55 100644 --- a/tests/unit/db/webhook.test.cpp +++ b/tests/unit/db/webhook.test.cpp @@ -157,7 +157,7 @@ LWS_CASE("db::storage::*_webhook") EXPECT(outs.size() == 1); lws::db::block_info head = last_block; - for (unsigned i = 0; i < 1; ++i) + for (unsigned i = 0; i < 4; ++i) { crypto::hash chain[2] = {head.hash, crypto::rand()}; @@ -184,24 +184,26 @@ LWS_CASE("db::storage::*_webhook") else EXPECT(updated->confirm_pubs.empty()); - full_account.updated(head.id); - head = {lws::db::block_id(lmdb::to_native(head.id) + 1), chain[1]}; + const auto next = lws::db::block_id(lmdb::to_native(head.id) + 1); + full_account.updated(next); + head = {next, chain[1]}; } } SECTION("storage::update(...) all at once") { - const crypto::hash chain[5] = { + const crypto::hash chain[6] = { last_block.hash, crypto::rand(), crypto::rand(), crypto::rand(), + crypto::rand(), crypto::rand() }; lws::account full_account = lws::db::test::make_account(account, view); full_account.updated(last_block.id); - EXPECT(add_out(full_account, last_block.id, 500)); + EXPECT(add_out(full_account, lws::db::block_id(lmdb::to_native(last_block.id) + 1), 500)); const std::vector outs = full_account.outputs(); EXPECT(outs.size() == 1); @@ -228,6 +230,61 @@ LWS_CASE("db::storage::*_webhook") EXPECT(updated->confirm_pubs[i].tx_info.payment_id.short_ == outs[0].payment_id.short_); } } + + SECTION("rescan with existing event") + { + crypto::hash chain[2] = { + last_block.hash, + crypto::rand() + }; + + lws::account full_account = lws::db::test::make_account(account, view); + full_account.updated(last_block.id); + EXPECT(add_out(full_account, last_block.id, 500)); + + const std::vector outs = full_account.outputs(); + EXPECT(outs.size() == 1); + + auto updated = db.update(last_block.id, chain, {std::addressof(full_account), 1}, nullptr); + EXPECT(updated.has_value()); + EXPECT(updated->spend_pubs.empty()); + EXPECT(updated->accounts_updated == 1); + EXPECT(updated->confirm_pubs.size() == 1); + + EXPECT(updated->confirm_pubs[0].key.user == lws::db::account_id(1)); + EXPECT(updated->confirm_pubs[0].key.type == lws::db::webhook_type::tx_confirmation); + EXPECT(updated->confirm_pubs[0].value.first.payment_id == 500); + EXPECT(updated->confirm_pubs[0].value.first.event_id == id); + EXPECT(updated->confirm_pubs[0].value.second.url == "http://the_url"); + EXPECT(updated->confirm_pubs[0].value.second.token == "the_token"); + EXPECT(updated->confirm_pubs[0].value.second.confirmations == 1); + + EXPECT(updated->confirm_pubs[0].tx_info.link == outs[0].link); + EXPECT(updated->confirm_pubs[0].tx_info.spend_meta.id == outs[0].spend_meta.id); + EXPECT(updated->confirm_pubs[0].tx_info.pub == outs[0].pub); + EXPECT(updated->confirm_pubs[0].tx_info.payment_id.short_ == outs[0].payment_id.short_); + + // issue a rescan, and ensure that + const std::size_t chain_size = std::end(chain) - std::begin(chain); + const auto new_height = lws::db::block_id(lmdb::to_native(last_block.id) - chain_size); + const auto rescanned = + db.rescan(new_height, {std::addressof(full_account.db_address()), 1}); + EXPECT(rescanned.has_value()); + EXPECT(rescanned->size() == 1); + + full_account.updated(new_height); + EXPECT(full_account.scan_height() == last_block.id); + + full_account = lws::db::test::make_account(account, view); + full_account.updated(new_height); + EXPECT(full_account.scan_height() == new_height); + updated = db.update(new_height, chain, {std::addressof(full_account), 1}, nullptr); + EXPECT(updated.has_value()); + EXPECT(updated->spend_pubs.empty()); + EXPECT(updated->accounts_updated == 1); + EXPECT(updated->confirm_pubs.size() == 0); + } + SECTION("Add db spend") { const boost::uuids::uuid other_id = boost::uuids::random_generator{}(); diff --git a/tests/unit/scanner.test.cpp b/tests/unit/scanner.test.cpp index e06c9d7..a2cf8c7 100644 --- a/tests/unit/scanner.test.cpp +++ b/tests/unit/scanner.test.cpp @@ -194,7 +194,7 @@ namespace transaction out{}; EXPECT( cryptonote::construct_tx_and_get_tx_key( - keys, subaddresses, sources, destinations, keys.m_account_address, {}, out.tx, 0, unused_key, + keys, subaddresses, sources, destinations, keys.m_account_address, {}, out.tx, /* 0, */ unused_key, out.additional_keys, true, {rct::RangeProofType::RangeProofBulletproof, 2}, use_view_tag ) );