[Fix #710] Use network_id to identify faucet_url

CHANGELOG.md

@@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 - Allow empty strings for the purpose of removing fields in DIDSet transaction
+- Refactor the API of `get_faucet_wallet` method. This produces a non-ambiguous URL for Testnet and Devnet faucet hosts.
 
 ## [2.6.0] - 2024-06-03
 

tests/unit/asyn/wallet/test_wallet.py

@@ -3,6 +3,7 @@
 from xrpl.asyncio.wallet.wallet_generation import (
     _DEV_FAUCET_URL,
     _TEST_FAUCET_URL,
+    XRPLFaucetException,
     get_faucet_url,
     process_faucet_host_url,
 )
@@ -16,30 +17,16 @@ def test_wallet_get_xaddress(self):
         expected = classic_address_to_xaddress(wallet.classic_address, None, False)
         self.assertEqual(wallet.get_xaddress(), expected)
 
-    def test_get_faucet_wallet_dev(self):
-        json_client_url = "https://s.devnet.rippletest.net:51234"
-        ws_client_url = "wss://s.devnet.rippletest.net/"
-        expected_faucet = _DEV_FAUCET_URL
+    def test_get_faucet_wallet_valid(self):
+        self.assertEqual(get_faucet_url(1), _TEST_FAUCET_URL)
+        self.assertEqual(get_faucet_url(2), _DEV_FAUCET_URL)
 
-        self.assertEqual(get_faucet_url(json_client_url), expected_faucet)
-        self.assertEqual(get_faucet_url(ws_client_url), expected_faucet)
+    def test_get_faucet_wallet_invalid(self):
+        with self.assertRaises(XRPLFaucetException):
+            get_faucet_url(0)  # corresponds to mainnet
 
-    def test_get_faucet_wallet_custom(self):
-        json_client_url = "https://s.devnet.rippletest.net:51234"
-        ws_client_url = "wss://s.devnet.rippletest.net/"
-        custom_host = "my_host.org"
-        expected_faucet = "https://my_host.org/accounts"
-
-        self.assertEqual(get_faucet_url(json_client_url, custom_host), expected_faucet)
-        self.assertEqual(get_faucet_url(ws_client_url, custom_host), expected_faucet)
-
-    def test_get_faucet_wallet_test(self):
-        json_client_url = "https://testnet.xrpl-labs.com"
-        ws_client_url = "wss://testnet.xrpl-labs.com"
-        expected_faucet = _TEST_FAUCET_URL
-
-        self.assertEqual(get_faucet_url(json_client_url), expected_faucet)
-        self.assertEqual(get_faucet_url(ws_client_url), expected_faucet)
+        with self.assertRaises(XRPLFaucetException):
+            get_faucet_url(-1)  # network_id must be non-negative
 
 
 class TestProcessFaucetHostURL(TestCase):

xrpl/asyncio/clients/client.py

@@ -7,6 +7,8 @@
 
 from typing_extensions import Final, Self
 
+from xrpl.asyncio.clients.exceptions import XRPLRequestFailureException
+from xrpl.models.requests import ServerInfo
 from xrpl.models.requests.request import Request
 from xrpl.models.response import Response
 
@@ -52,3 +54,28 @@ async def _request_impl(
         :meta private:
         """
         pass
+
+
+async def get_network_id_and_build_version(client: Client) -> None:
+    """
+    Get the network id and build version of the connected server.
+
+    Args:
+        client: The network client to use to send the request.
+
+    Raises:
+        XRPLRequestFailureException: if the rippled API call fails.
+    """
+    # the required values are already present, no need for further processing
+    if client.network_id and client.build_version:
+        return
+
+    response = await client._request_impl(ServerInfo())
+    if response.is_successful():
+        if "network_id" in response.result["info"]:
+            client.network_id = response.result["info"]["network_id"]
+        if not client.build_version and "build_version" in response.result["info"]:
+            client.build_version = response.result["info"]["build_version"]
+        return
+
+    raise XRPLRequestFailureException(response.result)

xrpl/asyncio/transaction/main.py

@@ -1,17 +1,19 @@
 """High-level transaction methods with XRPL transactions."""
+
 import math
 from typing import Any, Dict, Optional, cast
 
 from typing_extensions import Final
 
 from xrpl.asyncio.account import get_next_valid_seq_number
 from xrpl.asyncio.clients import Client, XRPLRequestFailureException
+from xrpl.asyncio.clients.client import get_network_id_and_build_version
 from xrpl.asyncio.ledger import get_fee, get_latest_validated_ledger_sequence
 from xrpl.constants import XRPLException
 from xrpl.core.addresscodec import is_valid_xaddress, xaddress_to_classic_address
 from xrpl.core.binarycodec import encode, encode_for_multisigning, encode_for_signing
 from xrpl.core.keypairs.main import sign as keypairs_sign
-from xrpl.models.requests import ServerInfo, ServerState, SubmitOnly
+from xrpl.models.requests import ServerState, SubmitOnly
 from xrpl.models.response import Response
 from xrpl.models.transactions import EscrowFinish
 from xrpl.models.transactions.transaction import Signer, Transaction
@@ -230,8 +232,7 @@ async def autofill(
         The autofilled transaction.
     """
     transaction_json = transaction.to_dict()
-    if not client.network_id:
-        await _get_network_id_and_build_version(client)
+    await get_network_id_and_build_version(client)
     if "network_id" not in transaction_json and _tx_needs_networkID(client):
         transaction_json["network_id"] = client.network_id
     if "sequence" not in transaction_json:
@@ -247,27 +248,6 @@ async def autofill(
     return Transaction.from_dict(transaction_json)
 
 
-async def _get_network_id_and_build_version(client: Client) -> None:
-    """
-    Get the network id and build version of the connected server.
-
-    Args:
-        client: The network client to use to send the request.
-
-    Raises:
-        XRPLRequestFailureException: if the rippled API call fails.
-    """
-    response = await client._request_impl(ServerInfo())
-    if response.is_successful():
-        if "network_id" in response.result["info"]:
-            client.network_id = response.result["info"]["network_id"]
-        if not client.build_version and "build_version" in response.result["info"]:
-            client.build_version = response.result["info"]["build_version"]
-        return
-
-    raise XRPLRequestFailureException(response.result)
-
-
 def _tx_needs_networkID(client: Client) -> bool:
     """
     Determines whether the transactions required network ID to be valid.

xrpl/asyncio/wallet/wallet_generation.py

@@ -1,4 +1,5 @@
 """Handles wallet generation from a faucet."""
+
 import asyncio
 from typing import Optional
 from urllib.parse import urlparse, urlunparse
@@ -8,6 +9,7 @@
 
 from xrpl.asyncio.account import get_balance, get_next_valid_seq_number
 from xrpl.asyncio.clients import Client, XRPLRequestFailureException
+from xrpl.asyncio.clients.client import get_network_id_and_build_version
 from xrpl.constants import XRPLException
 from xrpl.wallet.main import Wallet
 
@@ -57,7 +59,17 @@ async def generate_faucet_wallet(
 
     .. # noqa: DAR402 exception raised in private method
     """
-    faucet_url = get_faucet_url(client.url, faucet_host)
+    await get_network_id_and_build_version(client)
+
+    if faucet_host is not None:
+        faucet_url = process_faucet_host_url(faucet_host)
+    else:
+        if client.network_id is None:
+            raise XRPLFaucetException(
+                "Cannot create faucet URL without network_id or the faucet_host "
+                + "information"
+            )
+        faucet_url = get_faucet_url(client.network_id)
 
     if wallet is None:
         wallet = Wallet.create()
@@ -150,35 +162,32 @@ def process_faucet_host_url(input_url: str) -> str:
     return final_url
 
 
-def get_faucet_url(url: str, faucet_host: Optional[str] = None) -> str:
+def get_faucet_url(network_id: int) -> str:
     """
-    Returns the URL of the faucet that should be used, based on whether the URL is from
-    a testnet or devnet client.
+    Returns the URL of the faucet that should be used, based on network_id
 
     Args:
-        url: The URL that the client is using to access the ledger.
-        faucet_host: A custom host to use for funding a wallet.
+        network_id: The network_id corresponding to the XRPL network. This is parsed
+            from a ServerInfo() rippled response
 
     Returns:
         The URL of the matching faucet.
 
     Raises:
-        XRPLFaucetException: if the provided URL is not for the testnet or devnet.
+        XRPLFaucetException: if the provided network_id does not correspond to testnet
+            or devnet.
     """
-    if faucet_host is not None:
-        return process_faucet_host_url(faucet_host)
-    if "altnet" in url or "testnet" in url:  # testnet
+    if network_id == 1:
         return _TEST_FAUCET_URL
-    if "sidechain-net2" in url:  # sidechain issuing chain devnet
-        raise XRPLFaucetException(
-            "Cannot fund an account on an issuing chain. Accounts must be created via "
-            "the bridge."
-        )
-    if "devnet" in url:  # devnet
+    elif network_id == 2:
         return _DEV_FAUCET_URL
-    raise XRPLFaucetException(
-        "Cannot fund an account with a client that is not on the testnet or devnet."
-    )
+    elif network_id == 0:
+        raise XRPLFaucetException("Cannot create faucet with a client on mainnet.")
+    elif network_id < 0:
+        raise XRPLFaucetException("Unable to parse the specified network_id.")
+
+    # this line is unreachable. Custom devnets must specify a faucet_host input
+    return ""
 
 
 async def _check_wallet_balance(address: str, client: Client) -> int: