From 518c2e2c1918f85f9113bc8742eb1f2210be6468 Mon Sep 17 00:00:00 2001 From: Peter John Bushnell Date: Mon, 15 Apr 2024 09:23:06 +0100 Subject: [PATCH] Prevent empty token name in RPC (#2887) * Prevent empty token name in RPC * If no token name provided use symbol * Circular deps * Revert "If no token name provided use symbol" This reverts commit ffd3fd799dca8e8e54e761857b67b03bb56fb3b4. * Update errors on update token --------- Co-authored-by: Prasanna Loganathar --- src/dfi/rpc_tokens.cpp | 14 ++++- test/functional/feature_auth_return_change.py | 8 ++- test/functional/feature_token_split.py | 2 +- test/functional/feature_tokens_basic.py | 62 ++++++++++++++----- test/functional/test_runner.py | 1 + 5 files changed, 68 insertions(+), 19 deletions(-) diff --git a/src/dfi/rpc_tokens.cpp b/src/dfi/rpc_tokens.cpp index 838d35a28a..22fa1c9e74 100644 --- a/src/dfi/rpc_tokens.cpp +++ b/src/dfi/rpc_tokens.cpp @@ -114,6 +114,10 @@ UniValue createtoken(const JSONRPCRequest &request) { token.flags = metaObj["isDAT"].getBool() ? token.flags | (uint8_t)CToken::TokenFlags::DAT : token.flags; // setting isDAT + if (token.name.empty()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Token name should not be empty"); + } + if (!metaObj["tradeable"].isNull()) { token.flags = metaObj["tradeable"].getBool() ? token.flags | uint8_t(CToken::TokenFlags::Tradeable) : token.flags & ~uint8_t(CToken::TokenFlags::Tradeable); @@ -245,6 +249,10 @@ UniValue updatetoken(const JSONRPCRequest &request) { RPCTypeCheck(request.params, {UniValueType(), UniValue::VOBJ, UniValue::VARR}, true); // first means "any" const std::string tokenStr = trim_ws(request.params[0].getValStr()); + if (tokenStr.empty()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Token name cannot be empty")); + } + UniValue metaObj = request.params[1].get_obj(); const UniValue &txInputs = request.params[2]; @@ -256,12 +264,12 @@ UniValue updatetoken(const JSONRPCRequest &request) { LOCK(cs_main); DCT_ID id; auto token = pcustomcsview->GetTokenGuessId(tokenStr, id); - if (id == DCT_ID{0}) { - throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Can't alter DFI token!")); - } if (!token) { throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Token %s does not exist!", tokenStr)); } + if (id == DCT_ID{0}) { + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Can't alter DFI token!")); + } // Note: This is expected to be removed after DF23 if (Params().NetworkIDString() != CBaseChainParams::REGTEST && token->IsDAT()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot update DAT token"); diff --git a/test/functional/feature_auth_return_change.py b/test/functional/feature_auth_return_change.py index 2f297ede0c..b1715956ce 100755 --- a/test/functional/feature_auth_return_change.py +++ b/test/functional/feature_auth_return_change.py @@ -65,7 +65,12 @@ def run_test(self): # Create foundation token create_tx = self.nodes[0].createtoken( - {"symbol": "GOLD", "isDAT": False, "collateralAddress": collateral_a} + { + "symbol": "GOLD", + "name": "GOLD", + "isDAT": False, + "collateralAddress": collateral_a, + } ) self.nodes[0].generate(1, 1000000, coinbase) @@ -279,6 +284,7 @@ def run_test(self): create_tx = self.nodes[0].createtoken( { "symbol": "BRONZE", + "name": "BRONZE", "isDAT": True, "collateralAddress": self.nodes[0].PRIV_KEYS[0].ownerAuthAddress, } diff --git a/test/functional/feature_token_split.py b/test/functional/feature_token_split.py index fbac58449d..6b34dc4565 100755 --- a/test/functional/feature_token_split.py +++ b/test/functional/feature_token_split.py @@ -632,7 +632,7 @@ def token_split(self): -32600, "Invalid token symbol", self.nodes[0].createtoken, - {"symbol": "bad/v1", "collateralAddress": self.address}, + {"symbol": "bad/v1", "name": "bad", "collateralAddress": self.address}, ) # Set expected minted amount diff --git a/test/functional/feature_tokens_basic.py b/test/functional/feature_tokens_basic.py index 0a660c535a..7c34edd8ee 100755 --- a/test/functional/feature_tokens_basic.py +++ b/test/functional/feature_tokens_basic.py @@ -11,12 +11,12 @@ from test_framework.test_framework import DefiTestFramework from test_framework.authproxy import JSONRPCException -from test_framework.util import assert_equal +from test_framework.util import assert_equal, assert_raises_rpc_error class TokensBasicTest(DefiTestFramework): def set_test_params(self): - self.num_nodes = 3 + self.num_nodes = 2 # node0: main # node1: revert of destroy # node2: revert create (all) @@ -24,7 +24,6 @@ def set_test_params(self): self.extra_args = [ ["-txnotokens=0", "-amkheight=50"], ["-txnotokens=0", "-amkheight=50"], - ["-txnotokens=0", "-amkheight=50"], ] def run_test(self): @@ -33,27 +32,47 @@ def run_test(self): self.nodes[0].generate(100) self.sync_blocks() - # Stop node #2 for future revert - self.stop_node(2) - # CREATION: # ======================== collateral0 = self.nodes[0].getnewaddress("", "legacy") + # Try and create token without a name + assert_raises_rpc_error( + -8, + "Token name should not be empty", + self.nodes[0].createtoken, + { + "symbol": "GOLD", + "collateralAddress": collateral0, + }, + ) + + # Try and create token with an empty name + assert_raises_rpc_error( + -8, + "Token name should not be empty", + self.nodes[0].createtoken, + { + "symbol": "GOLD", + "name": "", + "collateralAddress": collateral0, + }, + ) + # Fail to create: Insufficient funds (not matured coins) try: - createTokenTx = self.nodes[0].createtoken( + self.nodes[0].createtoken( { "symbol": "GOLD", "name": "shiny gold", "collateralAddress": collateral0, - }, - [], + } ) except JSONRPCException as e: errorString = e.error["message"] assert "Insufficient funds" in errorString + # Mine a block to mature some coins self.nodes[0].generate(1) # Fail to create: use # in symbol @@ -63,17 +82,14 @@ def run_test(self): "symbol": "GOLD#1", "name": "shiny gold", "collateralAddress": collateral0, - }, - [], + } ) except JSONRPCException as e: errorString = e.error["message"] assert "Invalid token symbol" in errorString - print("Create token 'GOLD' (128)...") createTokenTx = self.nodes[0].createtoken( - {"symbol": "GOLD", "name": "shiny gold", "collateralAddress": collateral0}, - [], + {"symbol": "GOLD", "name": "shiny gold", "collateralAddress": collateral0} ) # Create and sign (only) collateral spending tx @@ -173,6 +189,24 @@ def run_test(self): assert_equal(t130["130"]["mintable"], False) assert_equal(t130["130"]["tradeable"], False) + # Try and update an empty token name + assert_raises_rpc_error( + -8, + "Token name cannot be empty", + self.nodes[0].updatetoken, + "", + {"isDAT": True}, + ) + + # Try and update an empty token name + assert_raises_rpc_error( + -8, + "Token NONEXISTANT does not exist!", + self.nodes[0].updatetoken, + "NONEXISTANT", + {"isDAT": True}, + ) + if __name__ == "__main__": TokensBasicTest().main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 1820fb92a3..762d452e77 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -195,6 +195,7 @@ "feature_poolswap.py", "feature_split_migrate_lock.py", "feature_poolswap_composite.py", + "feature_token_fractional_split.py", "feature_future_swap_limitation.py", "feature_poolswap_mechanism.py", "feature_poolswap_mainnet.py",