Skip to content

refactor: use NetworkID in faucet calculations #2950

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/xrpl/HISTORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@ Subscribe to [the **xrpl-announce** mailing list](https://groups.google.com/g/xr
## Unreleased

### Fixed
* `OracleSet` transaction accepts hexadecimal string values for `AssetPrice` field
* Fix `OracleSet` transaction to accept hexadecimal string values for `AssetPrice` field
* `TransactionStream` model includes `hash` field in APIv2
* `TransactionStream` model includes `close_time_iso` field only for APIv2
* Better faucet support

## 4.2.0 (2025-2-13)

Expand Down
35 changes: 19 additions & 16 deletions packages/xrpl/src/Wallet/defaultFaucets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,34 +16,37 @@ export enum FaucetNetwork {
Devnet = 'faucet.devnet.rippletest.net',
}

export const FaucetNetworkPaths: Record<string, string> = {
export const faucetNetworkPaths: Record<string, string> = {
[FaucetNetwork.Testnet]: '/accounts',
[FaucetNetwork.Devnet]: '/accounts',
}

export const faucetNetworkIDs: Map<number, FaucetNetwork> = new Map([
[1, FaucetNetwork.Testnet],
[2, FaucetNetwork.Devnet],
])

/**
* Get the faucet host based on the Client connection.
*
* @param client - Client.
* @returns A {@link FaucetNetwork}.
* @throws When the client url is not on altnet or devnet.
* @throws When there is no known faucet for the client's network ID.
*/
export function getFaucetHost(client: Client): FaucetNetwork | undefined {
const connectionUrl = client.url

// 'altnet' for Ripple Testnet server and 'testnet' for XRPL Labs Testnet server
if (connectionUrl.includes('altnet') || connectionUrl.includes('testnet')) {
return FaucetNetwork.Testnet
}

if (connectionUrl.includes('sidechain-net2')) {
if (client.networkID == null) {
throw new XRPLFaucetError(
'Cannot fund an account on an issuing chain. Accounts must be created via the bridge.',
'Cannot create faucet URL without networkID or the faucet_host information',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'Cannot create faucet URL without networkID or the faucet_host information',
'Cannot create faucet URL without networkID',

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you want to remove the extra clause? If someone provides the faucet_host, the function doesn't need the network ID.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How will faucet_host information help? This method will throw an error if client.networkID == null

)
}

if (connectionUrl.includes('devnet')) {
return FaucetNetwork.Devnet
if (faucetNetworkIDs.has(client.networkID)) {
return faucetNetworkIDs.get(client.networkID)
}

if (client.networkID === 0) {
// mainnet does not have a faucet
throw new XRPLFaucetError('Faucet is not available for mainnet.')
}

throw new XRPLFaucetError('Faucet URL is not defined or inferrable.')
Expand All @@ -54,11 +57,11 @@ export function getFaucetHost(client: Client): FaucetNetwork | undefined {
*
* @param hostname - hostname.
* @returns A String with the correct path for the input hostname.
* If hostname undefined or cannot find (key, value) pair in {@link FaucetNetworkPaths}, defaults to '/accounts'
* If hostname undefined or cannot find (key, value) pair in {@link faucetNetworkPaths}, defaults to '/accounts'
*/
export function getDefaultFaucetPath(hostname: string | undefined): string {
export function getFaucetPath(hostname: string | undefined): string {
if (hostname === undefined) {
return '/accounts'
}
return FaucetNetworkPaths[hostname] || '/accounts'
return faucetNetworkPaths[hostname] || '/accounts'
}
45 changes: 17 additions & 28 deletions packages/xrpl/src/Wallet/fundWallet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,7 @@ import { isValidClassicAddress } from 'ripple-address-codec'
import type { Client } from '../client'
import { XRPLFaucetError } from '../errors'

import {
FaucetWallet,
getFaucetHost,
getDefaultFaucetPath,
} from './defaultFaucets'
import { FaucetWallet, getFaucetHost, getFaucetPath } from './defaultFaucets'

import { Wallet } from '.'

Expand Down Expand Up @@ -148,7 +144,7 @@ export async function requestFunding(
if (!hostname) {
throw new XRPLFaucetError('No faucet hostname could be derived')
}
const pathname = options.faucetPath ?? getDefaultFaucetPath(hostname)
const pathname = options.faucetPath ?? getFaucetPath(hostname)
const response = await fetch(`https://${hostname}${pathname}`, {
method: 'POST',
headers: {
Expand Down Expand Up @@ -190,31 +186,24 @@ async function processSuccessfulResponse(
new XRPLFaucetError(`The faucet account is undefined`),
)
}
try {
// Check at regular interval if the address is enabled on the XRPL and funded
const updatedBalance = await getUpdatedBalance(
client,
classicAddress,
startingBalance,
)
// Check at regular interval if the address is enabled on the XRPL and funded
const updatedBalance = await getUpdatedBalance(
client,
classicAddress,
startingBalance,
)

if (updatedBalance > startingBalance) {
return {
wallet: walletToFund,
balance: updatedBalance,
}
if (updatedBalance > startingBalance) {
return {
wallet: walletToFund,
balance: updatedBalance,
}
throw new XRPLFaucetError(
`Unable to fund address with faucet after waiting ${
INTERVAL_SECONDS * MAX_ATTEMPTS
} seconds`,
)
} catch (err) {
if (err instanceof Error) {
throw new XRPLFaucetError(err.message)
}
throw err
}
throw new XRPLFaucetError(
`Unable to fund address with faucet after waiting ${
INTERVAL_SECONDS * MAX_ATTEMPTS
} seconds`,
)
Comment on lines +203 to +206
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error message is not relevant for the processSuccessfulResponse method. I do not see any retry-mechanism (or) back-off algorithm used here.

However, it is relevant for getUpdatedBalance method. These two error messages appear to have been swapped.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the errors are correct. The faucet can also be used to add funds to an existing wallet, in which case the error should fall through to here, not throw in getUpdatedBalance.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you point me to the code where we do the waiting (or) retry mechanism? Most of the usages of the faucet have been to generate a new wallet.

Even while funding an existing wallet, the operation is tried exactly once. I don't see the retry-operation.

}

async function processError(response: Response, body): Promise<never> {
Expand Down
34 changes: 14 additions & 20 deletions packages/xrpl/test/wallet/fundWallet.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ import { assert } from 'chai'

import {
FaucetNetwork,
FaucetNetworkPaths,
faucetNetworkPaths,
getFaucetHost,
getDefaultFaucetPath,
getFaucetPath,
} from '../../src/Wallet/defaultFaucets'
import {
setupClient,
Expand All @@ -22,47 +22,41 @@ describe('Get Faucet host ', function () {

it('returns the Devnet host', function () {
const expectedFaucet = FaucetNetwork.Devnet
// @ts-expect-error Intentionally modifying private data for test
testContext.client.connection.url = FaucetNetwork.Devnet
testContext.client.networkID = 2

assert.strictEqual(getFaucetHost(testContext.client), expectedFaucet)
})

it('returns the Testnet host', function () {
const expectedFaucet = FaucetNetwork.Testnet
// @ts-expect-error Intentionally modifying private data for test
testContext.client.connection.url = FaucetNetwork.Testnet

assert.strictEqual(getFaucetHost(testContext.client), expectedFaucet)
})

it('returns the Testnet host with the XRPL Labs server', function () {
const expectedFaucet = FaucetNetwork.Testnet
// @ts-expect-error Intentionally modifying private data for test
testContext.client.connection.url = 'wss://testnet.xrpl-labs.com'
testContext.client.networkID = 1

assert.strictEqual(getFaucetHost(testContext.client), expectedFaucet)
})

it('returns the correct faucetPath for Devnet host', function () {
const expectedFaucetPath = FaucetNetworkPaths[FaucetNetwork.Devnet]
// @ts-expect-error Intentionally modifying private data for test
testContext.client.connection.url = FaucetNetwork.Devnet
const expectedFaucetPath = faucetNetworkPaths[FaucetNetwork.Devnet]
testContext.client.networkID = 2

assert.strictEqual(
getDefaultFaucetPath(getFaucetHost(testContext.client)),
getFaucetPath(getFaucetHost(testContext.client)),
expectedFaucetPath,
)
})

it('returns the correct faucetPath for undefined host', function () {
const expectedFaucetPath = '/accounts'

assert.strictEqual(getDefaultFaucetPath(undefined), expectedFaucetPath)
assert.strictEqual(getFaucetPath(undefined), expectedFaucetPath)
})

it('throws if connected to mainnet', function () {
testContext.client.networkID = 0
assert.throws(() => getFaucetHost(testContext.client))
})

it('throws if not connected to a known faucet host', function () {
// Info: setupClient.setup creates a connection to 'localhost'
testContext.client.networkID = 300
assert.throws(() => getFaucetHost(testContext.client))
})
})
Loading