Feat(SMA-598): Smart Account Withdrawals #424

joepegler · 2024-02-20T10:55:47Z

Summary

Public utility method which withdraws funds from Smart Account to recipient (usually EOA)

Related Issue: SMA-598

Change Type

Checklist

My code follows this project's style guidelines
I've reviewed my own code
I've added comments for any hard-to-understand areas
I've updated the documentation if necessary
My changes generate no new warnings
I've added tests that prove my fix is effective or my feature works
All unit tests pass locally with my changes
Any dependent changes have been merged and published

Additional Information

Branch Naming

…lances

linear · 2024-02-20T11:21:36Z

SMA-598 smartAccount.withdraw()

livingrockrises · 2024-02-22T10:53:44Z

packages/account/src/BiconomySmartAccountV2.ts

+   * const { success } = await wait();
+   */
+  public async withdraw(
+    recipient: Hex,


we could also make recipient per each withdraw request

optionally. if not provided use the main recipient given above

They can always call withdraw multiple times too per recipient in a loop that'd have the same effect. Actually having a method named 'withdraw' that doesn't go to the owner is slightly confusing, but I couldn't think of a clear path to finding the recipient without it being provided by the dev. I would have hardcoded this if I could have...

Can you think of a way to reliably find the owner in this case? I think it would be best now that I'm thinking about it...

hmm thinking. we could have method single withdraw (which also acts as withdrawERC20)
and also withdrawMultipleERC20 and give them an option to provide receiver for each withdrawal.

these methods are risky though cause if we make mistake in sdk or choose to go rough then all tokens would be transferred to us

I mean

[ * { address: USDT, amount: BigInt(1) }

here they could also provide recipient address, instead of having common recipient..or maybe if not provided then fallback to common recipient given above.

I agree with you it could by default go to the owner in most cases where owner is another hot wallet metamask EOA. but they might want to send elsewhere or to a friend.

OK makes sense. Will add.

livingrockrises · 2024-02-22T10:55:11Z

packages/account/src/BiconomySmartAccountV2.ts

+   * const { wait } = await smartAccount.withdraw(
+   *  account.pubKey, // recipient
+   *  [
+   *    { address: USDT, amount: BigInt(1) },


Are they supposed to give wei value?
or just like symbol 'USDT' and amount 0.5 is possible?

we'd have to keep symbols and chain and token decimals mapping.
and then find out using current chainId..

No I'm not using symbols here, it's a can of worms (we'd need a database of ticker mappings, and it'd have to be 100% reliable) as there's no easy way to match a symbol to an address. Happy to revisit this if people ask for it though. As it is here the dev will need to know the addresses - I think that's fine.

Yes they give gwei value. In the tsdoc here I've made this clear. I also think this is what devs will expect. Doing the opposite is more likely to cause confusion

packages/account/src/utils/Types.ts

livingrockrises · 2024-02-22T10:58:27Z

packages/account/tests/account.e2e.spec.ts

+    const balanceOfSABefore = (await checkBalance(publicClient, smartAccountAddress)) as bigint;
+    const balanceOfRecipientBefore = (await checkBalance(publicClient, smartAccountOwner)) as bigint;
+
+    const { wait } = await smartAccount.withdraw(smartAccountOwner, undefined /* null or undefined or [] */, {


how do you choose the amount to withdraw if paymaster is not involved?

if a paymaster call fails, we would withdraw all which will fail on chain again

They could call the getBalances method :)

Can you elaborate on this second point? I don't follow

if I want to withdraw native tokens and max amount (say 1 Matic) what's there in SA
and if there is no paymaster or paymaster call fails then pnd goes '0x' which means account payas for gas in native tokens. so we can't transfer 1 and also pay 0.01 matic in gas

Yes that's right. I have thrown this error if a user does not provide his own amount in this case. dev will be expected to find his own amounts to withdraw if no paymaster is used. It'll be on the dev to find the right amount somehow

https://github.com/bcnmy/biconomy-client-sdk/pull/424/files#diff-78e7afa7e87e267fae298f597794c5ab45544d36cd675d4dd9aa843337bb652bR369

I'm basically telling the dev that it's up to them to pick the amount of dust they want to leave behind, assuming they're not using a paymaster.

livingrockrises

review i

livingrockrises

review ii

Auto stash before merge of "feature/SMA-599_balances" and "origin/develop" Add withdrawal tests jsdoc comments continued Fix comments comments continued Make `getBalance` available with no arguments rename tokenAdress -> address Throw when no amount + no paymaster is set Tweak comment

…thdrawal

VGabriel45 · 2024-04-01T07:27:27Z

@joepegler the withdraw method implementation seems to be missing

…thdrawal

livingrockrises · 2024-04-10T15:20:51Z

these are all new methods so let's send it and get more feedback.

joepegler added 7 commits February 14, 2024 14:23

Get smart account balances

da340fb

lint:fix

f8a1b2e

fix comments

b7e0d98

remove log

d8d2cb2

fix build

66eb3c4

continued

8a7eb0d

Merge remote-tracking branch 'origin/develop' into feature/SMA-599_ba…

7374c79

…lances

joepegler marked this pull request as ready for review February 20, 2024 11:00

joepegler changed the title ~~Smart Account Withdrawals~~ Feat(SMA-598): Smart Account Withdrawals Feb 20, 2024

joepegler requested review from livingrockrises and VGabriel45 February 20, 2024 11:21

joepegler mentioned this pull request Feb 20, 2024

Add sub pages for smart account methods bcnmy/docs#210

Closed

livingrockrises reviewed Feb 22, 2024

View reviewed changes

packages/account/src/utils/Types.ts Show resolved Hide resolved

livingrockrises reviewed Feb 22, 2024

View reviewed changes

livingrockrises requested a review from AmanRaj1608 February 25, 2024 23:22

livingrockrises approved these changes Feb 25, 2024

View reviewed changes

joepegler force-pushed the feature/SMA-598_withdrawal branch from fecb066 to e6cd7db Compare February 26, 2024 11:10

joepegler changed the base branch from feature/SMA-599_balances to develop February 26, 2024 11:10

joepegler added 7 commits February 26, 2024 11:14

Merge remote-tracking branch 'origin/develop' into feature/SMA-598_wi…

891dd98

…thdrawal

fix comment

b689dc9

Merge remote-tracking branch 'origin/develop' into feature/SMA-598_wi…

5264016

…thdrawal

default recipient

4bb17a7

default with empty string

fa30abb

Merge remote-tracking branch 'origin/develop' into feature/SMA-598_wi…

480e47f

…thdrawal

Merge branch 'develop' into feature/SMA-598_withdrawal

7a2bd59

joepegler added 3 commits March 1, 2024 11:12

usdt fix

e4f067c

Merge remote-tracking branch 'origin/develop' into feature/SMA-598_wi…

37ed719

…thdrawal

cont.

e2193ad

VGabriel45 approved these changes Mar 4, 2024

View reviewed changes

AmanRaj1608 approved these changes Mar 4, 2024

View reviewed changes

joepegler added 3 commits March 4, 2024 13:48

Merge remote-tracking branch 'origin/develop' into feature/SMA-598_wi…

01f4c4d

…thdrawal

reinclude publicClient

de02980

Merge remote-tracking branch 'origin/develop' into feature/SMA-598_wi…

eeb55fd

…thdrawal

joepegler marked this pull request as draft March 4, 2024 16:21

joepegler added 2 commits March 6, 2024 15:27

merge develop

a8af7fd

lint: fix

007b703

joepegler added 4 commits April 3, 2024 18:07

continued

28422f1

Merge branch 'develop' into feature/SMA-598_withdrawal

7a93d87

lint:fix

079def2

yarn lock

eaef063

joepegler marked this pull request as ready for review April 3, 2024 17:56

joepegler added 3 commits April 3, 2024 19:01

fix comment

44d2529

fix comment

cb4b60a

Merge remote-tracking branch 'origin/develop' into feature/SMA-598_wi…

050d542

…thdrawal

livingrockrises merged commit f7e5420 into develop Apr 10, 2024
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat(SMA-598): Smart Account Withdrawals #424

Feat(SMA-598): Smart Account Withdrawals #424

joepegler commented Feb 20, 2024

linear bot commented Feb 20, 2024

livingrockrises Feb 22, 2024

livingrockrises Feb 22, 2024

joepegler Feb 22, 2024 •

edited

Loading

livingrockrises Feb 25, 2024

livingrockrises Feb 25, 2024

joepegler Feb 26, 2024

livingrockrises Feb 22, 2024

livingrockrises Feb 22, 2024

joepegler Feb 22, 2024 •

edited

Loading

joepegler Feb 22, 2024

livingrockrises Feb 25, 2024

livingrockrises Feb 22, 2024

joepegler Feb 22, 2024

joepegler Feb 22, 2024

livingrockrises Feb 25, 2024

joepegler Feb 26, 2024 •

edited

Loading

joepegler Feb 26, 2024 •

edited

Loading

joepegler Feb 26, 2024

livingrockrises left a comment

livingrockrises left a comment

VGabriel45 commented Apr 1, 2024

livingrockrises commented Apr 10, 2024

Feat(SMA-598): Smart Account Withdrawals #424

Feat(SMA-598): Smart Account Withdrawals #424

Conversation

joepegler commented Feb 20, 2024

Summary

Change Type

Checklist

Additional Information

Branch Naming

linear bot commented Feb 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joepegler Feb 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joepegler Feb 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joepegler Feb 26, 2024 • edited Loading

Choose a reason for hiding this comment

joepegler Feb 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

livingrockrises left a comment

Choose a reason for hiding this comment

livingrockrises left a comment

Choose a reason for hiding this comment

VGabriel45 commented Apr 1, 2024

livingrockrises commented Apr 10, 2024

joepegler Feb 22, 2024 •

edited

Loading

joepegler Feb 22, 2024 •

edited

Loading

joepegler Feb 26, 2024 •

edited

Loading

joepegler Feb 26, 2024 •

edited

Loading