Merge pull request #443 from stellar/updateBannedAccountsLogic

Update banned accounts logic

Author: marta-lokhova

docs/software/commands.md

@@ -252,6 +252,18 @@ Most commands return their results in JSON format.
 * **bans**
   List current active bans
 
+* **banaccounts**
+  * `banaccounts`<br>
+    Lists the currently filtered account addresses as a JSON array.<br>
+  * `banaccounts?accountids=G_ADDRESS1,G_ADDRESS2,...`<br>
+    Overrides the set of filtered G-addresses at runtime (the
+    `FILTERED_G_ADDRESSES` configuration). Any transaction whose source account,
+    operation source account, fee-bump fee source, or (for Soroban transactions)
+    write footprint account entry matches an address in this list will be
+    rejected from the transaction queue.<br>
+  * `banaccounts?accountids=`<br>
+    Clears all filtered accounts.<br>
+
 * **checkdb**
   Triggers the instance to perform a background check of the database's state.
 

src/herder/Herder.h

@@ -213,6 +213,14 @@ class Herder
     // gets the upgrades that are scheduled by this node
     virtual std::string getUpgradesJson() = 0;
 
+    // Override the filtered G-addresses at runtime, updating both classic
+    // and soroban transaction queues.
+    virtual void
+    setFilteredAccounts(std::vector<std::string> const& addresses) = 0;
+
+    // Return the currently active filtered G-addresses.
+    virtual std::vector<std::string> getFilteredAccounts() const = 0;
+
     virtual void forceSCPStateIntoSyncWithLastClosedLedger() = 0;
 
     // helper function to craft an SCPValue

src/herder/HerderImpl.cpp

@@ -1608,6 +1608,32 @@ HerderImpl::getUpgradesJson()
     return mUpgrades.getParameters().toDebugJson(ls);
 }
 
+void
+HerderImpl::setFilteredAccounts(std::vector<std::string> const& addresses)
+{
+    UnorderedSet<AccountID> accounts;
+    for (auto const& addr : addresses)
+    {
+        accounts.emplace(KeyUtils::fromStrKey<PublicKey>(addr));
+    }
+    mTransactionQueue.setFilteredAccounts(accounts);
+    if (mSorobanTransactionQueue)
+    {
+        mSorobanTransactionQueue->setFilteredAccounts(accounts);
+    }
+}
+
+std::vector<std::string>
+HerderImpl::getFilteredAccounts() const
+{
+    std::vector<std::string> result;
+    for (auto const& acc : mTransactionQueue.getFilteredAccounts())
+    {
+        result.push_back(KeyUtils::toStrKey(acc));
+    }
+    return result;
+}
+
 void
 HerderImpl::forceSCPStateIntoSyncWithLastClosedLedger()
 {

src/herder/HerderImpl.h

@@ -191,6 +191,11 @@ class HerderImpl : public Herder
     void setUpgrades(Upgrades::UpgradeParameters const& upgrades) override;
     std::string getUpgradesJson() override;
 
+    void
+    setFilteredAccounts(std::vector<std::string> const& addresses) override;
+
+    std::vector<std::string> getFilteredAccounts() const override;
+
     void forceSCPStateIntoSyncWithLastClosedLedger() override;
 
     bool resolveNodeID(std::string const& s, PublicKey& retKey) override;

src/herder/TransactionQueue.cpp

@@ -118,6 +118,18 @@ TransactionQueue::TransactionQueue(Application& app, uint32 pendingDepth,
         rand_uniform<uint64>(0, std::numeric_limits<uint64>::max());
 }
 
+void
+TransactionQueue::setFilteredAccounts(UnorderedSet<AccountID> const& accounts)
+{
+    mFilteredAccounts = accounts;
+}
+
+UnorderedSet<AccountID> const&
+TransactionQueue::getFilteredAccounts() const
+{
+    return mFilteredAccounts;
+}
+
 ClassicTransactionQueue::ClassicTransactionQueue(Application& app,
                                                  uint32 pendingDepth,
                                                  uint32 banDepth,

src/herder/TransactionQueue.h

@@ -162,6 +162,9 @@ class TransactionQueue
     TxFrameList getTransactions(LedgerHeader const& lcl) const;
     bool sourceAccountPending(AccountID const& accountID) const;
 
+    void setFilteredAccounts(UnorderedSet<AccountID> const& accounts);
+    UnorderedSet<AccountID> const& getFilteredAccounts() const;
+
     virtual size_t getMaxQueueSizeOps() const = 0;
 
 #ifdef BUILD_TESTS

src/herder/test/HerderTests.cpp

@@ -6030,6 +6030,163 @@ TEST_CASE("filter transactions by G address", "[herder]")
     }
 }
 
+TEST_CASE("banaccounts HTTP command", "[herder]")
+{
+    SECTION("ban accounts via command")
+    {
+        VirtualClock clock;
+        auto cfg = getTestConfig();
+        cfg.FILTERED_G_ADDRESSES = {};
+        Application::pointer app = createTestApplication(clock, cfg);
+
+        auto root = app->getRoot();
+        auto srcKey = SecretKey::pseudoRandomForTesting();
+        auto src = root->create(srcKey, 1000000000);
+
+        // Initially, no filter — transaction should be accepted
+        auto acc = getAccount("acc");
+        auto tx = src.tx({createAccount(acc.getPublicKey(), 1)});
+        REQUIRE(app->getHerder().recvTransaction(tx, false).code ==
+                TransactionQueue::AddResultCode::ADD_STATUS_PENDING);
+
+        // Now set the filter via the HTTP command
+        auto addr = KeyUtils::toStrKey(srcKey.getPublicKey());
+        auto result = app->getCommandHandler().manualCmd(
+            "banaccounts?accountids=" + addr);
+        REQUIRE(result.find("filtered accounts updated") != std::string::npos);
+        REQUIRE(result.find("\"count\": 1") != std::string::npos);
+
+        // New transaction from the same source should now be filtered
+        auto acc2 = getAccount("acc2");
+        auto tx2 = src.tx({createAccount(acc2.getPublicKey(), 1)});
+        REQUIRE(app->getHerder().recvTransaction(tx2, false).code ==
+                TransactionQueue::AddResultCode::ADD_STATUS_FILTERED);
+    }
+
+    SECTION("clear banned accounts via command")
+    {
+        VirtualClock clock;
+        auto cfg = getTestConfig();
+        auto srcKey = SecretKey::pseudoRandomForTesting();
+        cfg.FILTERED_G_ADDRESSES = {KeyUtils::toStrKey(srcKey.getPublicKey())};
+        Application::pointer app = createTestApplication(clock, cfg);
+
+        auto root = app->getRoot();
+        auto src = root->create(srcKey, 1000000000);
+
+        // Source is initially filtered
+        auto acc = getAccount("acc");
+        auto tx = src.tx({createAccount(acc.getPublicKey(), 1)});
+        REQUIRE(app->getHerder().recvTransaction(tx, false).code ==
+                TransactionQueue::AddResultCode::ADD_STATUS_FILTERED);
+
+        // Clear the filter via empty accountids
+        auto result = app->getCommandHandler().manualCmd(
+            "banaccounts?accountids=");
+        REQUIRE(result.find("filtered accounts cleared") != std::string::npos);
+
+        // Resubmit the same transaction — it should now be accepted
+        REQUIRE(app->getHerder().recvTransaction(tx, false).code ==
+                TransactionQueue::AddResultCode::ADD_STATUS_PENDING);
+    }
+
+    SECTION("multiple accounts")
+    {
+        VirtualClock clock;
+        auto cfg = getTestConfig();
+        cfg.FILTERED_G_ADDRESSES = {};
+        Application::pointer app = createTestApplication(clock, cfg);
+
+        auto root = app->getRoot();
+        auto key1 = SecretKey::pseudoRandomForTesting();
+        auto key2 = SecretKey::pseudoRandomForTesting();
+        auto src1 = root->create(key1, 1000000000);
+        auto src2 = root->create(key2, 1000000000);
+
+        auto addr1 = KeyUtils::toStrKey(key1.getPublicKey());
+        auto addr2 = KeyUtils::toStrKey(key2.getPublicKey());
+
+        auto result = app->getCommandHandler().manualCmd(
+            "banaccounts?accountids=" + addr1 + "," + addr2);
+        REQUIRE(result.find("\"count\": 2") != std::string::npos);
+
+        auto acc = getAccount("acc");
+        auto tx1 = src1.tx({createAccount(acc.getPublicKey(), 1)});
+        REQUIRE(app->getHerder().recvTransaction(tx1, false).code ==
+                TransactionQueue::AddResultCode::ADD_STATUS_FILTERED);
+
+        auto acc2 = getAccount("acc2");
+        auto tx2 = src2.tx({createAccount(acc2.getPublicKey(), 1)});
+        REQUIRE(app->getHerder().recvTransaction(tx2, false).code ==
+                TransactionQueue::AddResultCode::ADD_STATUS_FILTERED);
+    }
+
+    SECTION("invalid address returns error")
+    {
+        VirtualClock clock;
+        auto cfg = getTestConfig();
+        cfg.FILTERED_G_ADDRESSES = {};
+        Application::pointer app = createTestApplication(clock, cfg);
+
+        auto result = app->getCommandHandler().manualCmd(
+            "banaccounts?accountids=NOT_A_VALID_ADDRESS");
+        REQUIRE(result.find("invalid address") != std::string::npos);
+    }
+
+    SECTION("no accountids lists current filter")
+    {
+        VirtualClock clock;
+        auto cfg = getTestConfig();
+        auto srcKey = SecretKey::pseudoRandomForTesting();
+        auto addr = KeyUtils::toStrKey(srcKey.getPublicKey());
+        cfg.FILTERED_G_ADDRESSES = {addr};
+        Application::pointer app = createTestApplication(clock, cfg);
+
+        // With no accountids param, should list the current filter
+        auto result = app->getCommandHandler().manualCmd("banaccounts");
+        REQUIRE(result.find("filteredAccounts") != std::string::npos);
+        REQUIRE(result.find(addr) != std::string::npos);
+
+        // Set two accounts, then list again
+        auto key2 = SecretKey::pseudoRandomForTesting();
+        auto addr2 = KeyUtils::toStrKey(key2.getPublicKey());
+        app->getCommandHandler().manualCmd(
+            "banaccounts?accountids=" + addr + "," + addr2);
+
+        result = app->getCommandHandler().manualCmd("banaccounts");
+        REQUIRE(result.find(addr) != std::string::npos);
+        REQUIRE(result.find(addr2) != std::string::npos);
+
+        // Clear and verify empty list
+        app->getCommandHandler().manualCmd("banaccounts?accountids=");
+        result = app->getCommandHandler().manualCmd("banaccounts");
+        REQUIRE(result.find("filteredAccounts") != std::string::npos);
+        REQUIRE(result.find(addr) == std::string::npos);
+    }
+
+    SECTION("fee-bump with banned fee source is rejected")
+    {
+        VirtualClock clock;
+        auto cfg = getTestConfig();
+        cfg.FILTERED_G_ADDRESSES = {};
+        Application::pointer app = createTestApplication(clock, cfg);
+
+        auto root = app->getRoot();
+        auto filteredKey = SecretKey::pseudoRandomForTesting();
+        auto feeSource = root->create(filteredKey, 1000000000);
+        auto feeSourceAcct = TestAccount{*app, filteredKey};
+
+        auto addr = KeyUtils::toStrKey(filteredKey.getPublicKey());
+        app->getCommandHandler().manualCmd("banaccounts?accountids=" + addr);
+
+        auto innerTx = root->tx({payment(root->getPublicKey(), 1)});
+        auto fb = feeBump(*app, feeSourceAcct, innerTx, 200);
+
+        REQUIRE(app->getHerder().recvTransaction(fb, false).code ==
+                TransactionQueue::AddResultCode::ADD_STATUS_FILTERED);
+    }
+}
+
 // Test that Herder updates the scphistory table with additional messages from
 // ledger `n-1` when closing ledger `n`
 TEST_CASE("SCP message capture from previous ledger", "[herder]")

src/main/CommandHandler.cpp

@@ -43,6 +43,7 @@
 #include "test/TxTests.h"
 #endif
 #include <optional>
+#include <sstream>
 
 using std::placeholders::_1;
 using std::placeholders::_2;
@@ -123,6 +124,7 @@ CommandHandler::CommandHandler(Application& app) : mApp(app)
     addRoute("metrics", &CommandHandler::metrics);
     addRoute("tx", &CommandHandler::tx);
     addRoute("upgrades", &CommandHandler::upgrades);
+    addRoute("banaccounts", &CommandHandler::banaccounts);
     addRoute("dumpproposedsettings", &CommandHandler::dumpProposedSettings);
     addRoute("self-check", &CommandHandler::selfCheck);
     addRoute("sorobaninfo", &CommandHandler::sorobanInfo);
@@ -667,6 +669,65 @@ CommandHandler::upgrades(std::string const& params, std::string& retStr)
     }
 }
 
+void
+CommandHandler::banaccounts(std::string const& params, std::string& retStr)
+{
+    ZoneScoped;
+    std::map<std::string, std::string> retMap;
+    http::server::server::parseParams(params, retMap);
+
+    auto it = retMap.find("accountids");
+    if (it == retMap.end())
+    {
+        // parseParams drops empty values, so check the raw params to
+        // distinguish "not specified" (list) from "empty value" (clear).
+        if (params.find("accountids") != std::string::npos)
+        {
+            mApp.getHerder().setFilteredAccounts({});
+            retStr = R"({"status": "filtered accounts cleared"})";
+            return;
+        }
+
+        // No accountids param at all: list current filtered accounts
+        auto accounts = mApp.getHerder().getFilteredAccounts();
+        Json::Value root;
+        root["filteredAccounts"] = Json::arrayValue;
+        for (auto const& addr : accounts)
+        {
+            root["filteredAccounts"].append(addr);
+        }
+        retStr = root.toStyledString();
+        return;
+    }
+
+    std::vector<std::string> addresses;
+    std::istringstream iss(it->second);
+    std::string addr;
+    while (std::getline(iss, addr, ','))
+    {
+        if (addr.empty())
+        {
+            continue;
+        }
+        try
+        {
+            KeyUtils::fromStrKey<PublicKey>(addr);
+        }
+        catch (std::exception const&)
+        {
+            retStr = fmt::format(
+                FMT_STRING(R"({{"error": "invalid address: '{}'"}})"), addr);
+            return;
+        }
+        addresses.push_back(addr);
+    }
+
+    mApp.getHerder().setFilteredAccounts(addresses);
+    retStr = fmt::format(
+        FMT_STRING(R"({{"status": "filtered accounts updated", "count": {}}})"),
+        addresses.size());
+}
+
 void
 CommandHandler::dumpProposedSettings(std::string const& params,
                                      std::string& retStr)

src/main/CommandHandler.h

@@ -66,6 +66,7 @@ class CommandHandler
     void tx(std::string const& params, std::string& retStr);
     void unban(std::string const& params, std::string& retStr);
     void upgrades(std::string const& params, std::string& retStr);
+    void banaccounts(std::string const& params, std::string& retStr);
     void dumpProposedSettings(std::string const& params, std::string& retStr);
     void surveyTopology(std::string const&, std::string& retStr);
     void stopSurvey(std::string const&, std::string& retStr);

src/main/Config.cpp

@@ -354,7 +354,6 @@ Config::Config() : NODE_SEED(SecretKey::random())
     FILTERED_G_ADDRESSES = {
         "GBO7VUL2TOKPWFAWKATIW7K3QYA7WQ63VDY5CAE6AFUUX6BHZBOC2WXC",
         "GATDQL767ZM2JQTBEG4BQ5WKOQNGAGWZDUN4GYT2UINPEU3RT2UAMVZH",
-        "GC2XJKN5VZEMM35F5LRSUP5CWVDZJVM37YKR7UYYXGN3TGKZXMP5FZIB",
         "GCQCWEQDICASV3R737LPWPDJ3FPBC6XPWXKPJDL22DLQVGOJAUH5DBJI"};
 
     OP_APPLY_SLEEP_TIME_DURATION_FOR_TESTING = {};