Closed chadoh closed 3 weeks ago
Friendbot doesn't actually allow funding existing accounts, so behind the scenes this will need to create a new account and transfer all of its balance to the desired account.
It requires some discussion, probably in an issue on the stellar/go repo, but we can change friendbot so it better serves developers. I think we should open an issue about funding existing accounts, which wouldn't be difficult (🤞🏻) to add to friendbot.
The codebase for friendbot is here: https://github.com/stellar/go/tree/master/services/friendbot
Similar to @fnando's comment at https://github.com/stellar/stellar-cli/issues/1380#issuecomment-2204479337 I don't think we should add --minimum-balance
and instead just have fund always fund if the balance is below the amount that friendbot will send. i.e. if the balance is below 10000, deposit 10000. Friendbot can do that check. So the fund function makes one guarantee, the bal is at least the amount friendbot funds if successful.
My PR adds (10,000 XLM - current_balance). This was because if I just added 10,000 XLM, I would always get an error: topics:[error, Error(Contract, #10)], data:[\"resulting balance is not within the allowed range\", 10000000, 0, 9223372036854775807]
Not sure if that is a bug in the asset contract or if 10k XLM is just intended to be a hard limit. But the workaround was to just top it off up to 10k
I think we should add this functionality to friendbot not the CLI, so more than just the CLI benefit from the change, and to a lesser degree so that funding doesn't use 2x the ledger space and take 10-14 seconds, but a single ledger and 5-7 seconds. The friendbot change is a pretty small change, and I understand it's a bit left field of a contribution to make and ramp up on, so I've gone ahead and opened the change here:
@leighmcculloch when will https://github.com/stellar/go/pull/5399 be incorporated into quickstart? Deployed to futurenet & testnet? I'd like to keep this issue around until we can try out the CLI with those changes to see if we need to update any CLI logic. I think error handling in our fund_address
logic might need to be updated, at a minimum.
Good call:
Does the CLI utilize the new friendbot functionality or is that something we will need to add in a separate PR? @leighmcculloch
Yup, the cli fund command will top up account that drops below 10k. And errors will be communicated back to the user.
If the user tries to fund when Friendbot won't because its balance is too high the error from Friendbot will propagate to the command line.
It might be a better UX to ignore that error case in the cli and treat it like the success case, since even though Friendbot didn't fund more the account is still funded because it has at least 10k balance.
Thoughts?
I agree, I think it should ignore the error if the user already has 10k or more and perhaps just communicate that to the user - "Balance already above 10k, nothing to do" or something like that. Thoughts, @chadoh ?
What problem does your feature solve?
Right now if I have an existing funded account, let's call it
alice
, if I runkeys fund alice
I will get an error:What would you like to see?
I have always found this error message a little odd. I know that the account exists! Or rather, it's not important to me whether it exists or not. If the account already exists and the balance is above some number of XLM (maybe 100?), then everything's good. It can tell me that.
If funding occurs:
The maximum
--minimum-balance
should probably be 1000, since that's what Friendbot dishes out to new accounts.Friendbot doesn't actually allow funding existing accounts, so behind the scenes this will need to create a new account and transfer all of its balance to the desired account.
What alternatives are there?
We could just not? Make people top off long-lived test accounts themselves, or keep generating new ones.