-
Notifications
You must be signed in to change notification settings - Fork 340
fix(rpc): support contract addresses in requestAirdrop() #1314
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
fix(rpc): support contract addresses in requestAirdrop() #1314
Conversation
requestAirdrop() now properly supports C... (contract) addresses by detecting them via StrKey.isValidContract() and returning a ContractFundingResult instead of attempting to find a created account. Fixes funding contract addresses which previously threw 'No account created in transaction' error.
|
I am hesitant to have requestAirdrop return two different types. Since this will be a breaking change my thought was to just return the |
Yeah, that makes sense! It could look something like this: public async requestAirdrop(
address: string | Pick<Account, "accountId">,
friendbotUrl?: string,
): Promise<Api.GetTransactionResponse> {
const account = typeof address === "string" ? address : address.accountId();
friendbotUrl = friendbotUrl || (await this.getNetwork()).friendbotUrl;
if (!friendbotUrl) {
throw new Error("No friendbot URL configured for current network");
}
const response = await this.httpClient.post<FriendbotApi.Response>(
`${friendbotUrl}?addr=${encodeURIComponent(account)}`,
);
// Return full transaction response for both G... and C... addresses
return this.getTransaction(response.data.hash);
}Usage would then be: const tx = await server.requestAirdrop(address);
if (tx.status === Api.GetTransactionStatus.SUCCESS) {
console.log("Funded:", tx.txHash);
}
// If user needs Account object for G... addresses:
const account = await server.getAccount(address);This simplifies the implementation and gives users more flexibility with the full transaction details. Happy to update the PR if you'd like to go this direction. |
|
Hey @joaquinsoza sorry for the delay I've thought about it some more and think it would be better to go the route of keeping Would you be interested in updating the PR to reflect this direction? If not, I’m happy to take it over and implement the changes myself. |
|
Yeah I’m inclined to agree with @Ryang-21 here: we don’t want to break every existing |
contracts - Add fundAddress() that supports both G... and C... addresses - Returns GetSuccessfulTransactionResponse for consistent return type - Mark requestAirdrop() as deprecated - Add unit tests for fundAddress()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request adds a new fundAddress() method to support funding both Stellar account (G...) and contract (C...) addresses via the Friendbot faucet. The existing requestAirdrop() method is deprecated in favor of this new method.
Changes:
- Added new
fundAddress()method that returns transaction response instead of Account object - Deprecated
requestAirdrop()with a note directing users to the new method - Added comprehensive test suite for the new method covering various scenarios
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/rpc/server.ts | Added fundAddress() method and deprecated requestAirdrop() |
| test/unit/server/soroban/request_airdrop.test.ts | Added test suite for fundAddress() with tests for accounts, contracts, custom URLs, and error cases |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Thanks for the feedback @Ryang-21 @Shaptic! I've updated the PR with the suggested approach: Changes:
Usage: // Fund any address (account or contract)
const tx = await server.fundAddress(address);
console.log("Funded! Hash:", tx.txHash);
// If you need an Account object for G... addresses:
const account = await server.getAccount(address); Regarding creating an Account object from the result - I documented that users can call |
- Add input validation for G.../C... addresses using StrKey - Enhance error message to include transaction status on failure - Fix test assertion to match exact error message - Add test coverage for HTTP request failures - Add test coverage for invalid address inputs
Wrap HTTP request in try-catch to provide cleaner error messages when friendbot returns errors (e.g., when funding an already-funded address).
Ryang-21
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm appreciate the contribution!
Summary
Adds support for funding contract addresses (C...) via
requestAirdrop().Changes
StrKey.isValidContract()ContractFundingResultfor contracts instead of parsing account metadataApi.ContractFundingResulttypeFixes #1313