Withdrawals #474

r-czajkowski · 2024-06-14T08:34:55Z

Depends on: #472
Closes: #353

This PR adds support for withdrawals in SDK and Acre dapp. To request a redemption we need to implement our custom redeemer proxy. Our redeemer proxy builds the redemption data and sends the redemption request transaction via Orangekit SDK.

Here we also hardcode the ethers version to 6.10.0 (without ^) in the solidity and sdk workspaces. The version of ethers in the solidity workspace has been resolved to 6.13.1 and the sdk workspace depends on it, so pnpm resolves ethers in sdk to 6.13.1 under the hood where there are some changes in the API and causes tests and builds to fail.

To use the latest redeption features.

We need the address of the `BitcoinRedeemer` contract to initialize the withdrawal.

We need to sign the Bitcoin message in withdrawal flow.

Add `getChainIdentifier` and `encodeApproveAndCallFunctionData` methods. We need the stBTC contract address and encoded function data to initialize the withdrawal.

The `OrangeKitTbtcRedeemerProxy` requests redemption using the orangekit SDK. It builds necessary data for the orangekit safe transaction and sends transaction using the orangeKit.

Add `getPublicKey` method - we need the bitcoin account public key to interact with the orangekit.

The account module exposes function that initializes the withdrawal - it expects Bitcoin amount in satoshi precision. In this case we need to convert satoshi to stBTC shares. Also we need to estimate the amount of tBTC released after redeeming stBTC shares including all Acre fees using the `previewRedeem` stBTC contract function. The value is required by the tBTC SDK to determine the tBTC Wallet from which the Bitcoin will be bridged to the depositor's Bitcoin address.

Add unit test for `initiateRedemption` method.

Add unit tests for `initializeWithdrawal` function.

Looks like the `ethers` dependency has been upgraded to `6.13.0` and the interface has changed.

Use `v6.10.0` in `solidity` and `sdk` workspaces. The pnpm resolves the `ethers` in SDK to the version used in `solidity` workspace.

We need the gelato API to create the orangekit SDK.

nkuba · 2024-06-19T08:48:51Z

sdk/src/acre.ts

+// TODO: Should we hide this API key in SDK impl or should it be passed by the
+// consumer?


Let's pass it and expect it to be configured as dApp's env variable.

nkuba · 2024-06-19T08:51:21Z

sdk/src/lib/bitcoin/providers/provider.ts

@@ -14,4 +14,8 @@ export interface BitcoinProvider {
   * @returns Bitcoin address selected by the user.


Can we take the chance and remove this BitcoinProvider interface and use the one defined in OrangeKit?

Yep but we need to update the OrangeKit first. The BitcoinProvider lives in @orangekit/react. I think we should move this interface to the @orangekit/sdk.

Non-blocking. I created an issue to track it #489

sdk/src/lib/contracts/stbtc.ts

sdk/src/lib/ethereum/stbtc.ts

nkuba · 2024-06-19T08:57:15Z

sdk/src/lib/ethereum/stbtc.ts

+   */
+  encodeApproveAndCallFunctionData(
+    spender: ChainIdentifier,
+    amount: bigint,


Suggested change

amount: bigint,

shares: bigint,

nkuba · 2024-06-19T08:58:10Z

sdk/src/lib/contracts/stbtc.ts

+
+  encodeApproveAndCallFunctionData(
+    spender: ChainIdentifier,
+    amount: bigint,


Suggested change

amount: bigint,

shares: bigint,

nkuba · 2024-06-19T08:58:18Z

sdk/src/lib/contracts/stbtc.ts

+   * of stBTC shares.
+   * @param amount Amount of stBTC shares to redeem.
+   */
+  previewRedeem(amount: bigint): Promise<bigint>


Suggested change

previewRedeem(amount: bigint): Promise<bigint>

previewRedeem(shares: bigint): Promise<bigint>

nkuba · 2024-06-19T09:01:41Z

sdk/src/lib/redeemer-proxy.ts

+  static init(
+    contracts: AcreContracts,
+    orangeKitSdk: OrangeKitSdk,
+    account: {
+      publicKey: string
+      bitcoinAddress: string
+      ethereumAddress: ChainIdentifier
+    },
+    bitcoinProvider: BitcoinProvider,
+    sharesAmount: bigint,
+  ) {
+    return new OrangeKitTbtcRedeemerProxy(
+      contracts,
+      orangeKitSdk,
+      account,
+      bitcoinProvider,
+      sharesAmount,
+    )
+  }
+
+  private constructor(
+    contracts: AcreContracts,
+    orangeKitSdk: OrangeKitSdk,
+    account: {
+      publicKey: string
+      bitcoinAddress: string
+      ethereumAddress: ChainIdentifier
+    },
+    bitcoinProvider: BitcoinProvider,
+    sharesAmount: bigint,
+  ) {
+    this.#contracts = contracts
+    this.#orangeKitSdk = orangeKitSdk
+    this.#account = account
+    this.#bitcoinProvider = bitcoinProvider
+    this.#sharesAmount = sharesAmount
+  }


Do we need to have a separate constructor and init method? Looks like the constructor would be enough.

sdk/src/modules/tbtc/Tbtc.ts

nkuba · 2024-06-19T09:14:11Z

sdk/src/modules/tbtc/Tbtc.ts

+  async initiateRedemption(
+    destinationBitcoinAddress: string,
+    tbtcAmount: bigint,
+    redeemer: RedeemerProxy,


Suggested change

redeemer: RedeemerProxy,

redeemerProxy: RedeemerProxy,

Remove unnecessary `init` fn - use constructor instead.

dapp/src/hooks/sdk/useInitializeAcreSdk.ts

sdk/package.json

nkuba · 2024-06-20T20:40:51Z

sdk/src/lib/bitcoin/providers/provider.ts

@@ -14,4 +14,8 @@ export interface BitcoinProvider {
   * @returns Bitcoin address selected by the user.


Non-blocking. I created an issue to track it #489

sdk/test/utils/mock-orangekit.ts

We want to know when the user needs to sign the message and when he signed it, so here we add callbacks that are triggered before and after the signing process.

Display more information about a given process to the user. To initialize withdrawal we need to find a wallet that can hanlde the tBTC redemption request - it's time-consuming so we display to the user that we are building the transacion data. The next phase is sign message step - we display a message saying please finalize the signing process in yout wallet. The last step is waiting for the transacion so we are saying that withdrawal initialiazation is in progress.

To not repeat them.

Hardcode he Gelato api key in `AcreSdkContext` for consistency.

kkosiorowska

I left a few smaller comments, but I also noticed a few things.

When the user rejects signing the message should see ResumeModal. When the user resumes withdrawals has to wait until the transaction data builds again. I wonder if we can somehow avoid this.

Screen.Recording.2024-06-21.at.14.21.57.mov

It seems to me that even if we stop the process by clicking the "Cancel" button underneath we are still building data. This means that even though the user has closed the modal after some time a wallet window will appear to sign the message.

dapp/src/acre-react/hooks/useInitializeWithdraw.ts

dapp/src/components/TransactionModal/ActiveUnstakingStep/UnstakeFormModal/UnstakeDetails.tsx

dapp/src/hooks/useMinWithdrawAmount.ts

dapp/src/hooks/useTransactionDetails.ts

dapp/src/hooks/useTransactionFee.ts

dapp/src/components/TransactionModal/ActiveUnstakingStep/SignMessageModal.tsx

Use const variable instead of hardcoded value.

r-czajkowski · 2024-06-24T09:24:01Z

I left a few smaller comments, but I also noticed a few things.

When the user rejects signing the message should see ResumeModal. When the user resumes withdrawals has to wait until the transaction data builds again. I wonder if we can somehow avoid this.

Hmm I think we can't. This is limited by the tbtc-v2.ts lib. To achieve this we should refactor the tbtc-v2.ts and update the redemptions to be built in the same way as the deposit feature is built. cc @nkuba

r-czajkowski · 2024-06-24T09:37:40Z

It seems to me that even if we stop the process by clicking the "Cancel" button underneath we are still building data. This means that even though the user has closed the modal after some time a wallet window will appear to sign the message.

We can try to use AbortController and add the signal param to the initializeWithdrawal function. cc @nkuba

We should display the resume modal when the user rejects the sign message step. Otherwsie we want to display the error modal.

kkosiorowska

LGTM from the dApp perspective. 🚀 However, I leave the final approval to @nkuba.

kkosiorowska

LGTM, Talked with Kuba and let's no wait for it any longer. 🚀

Refs: #447 Depends on: #474 In this PR we implement withdrawal fee calculation in a way that matches the stBTC contract, where fee is calculated based on the total tBTC amount and rounded up.

r-czajkowski added 9 commits June 14, 2024 10:31

Bump tbtc-v2.ts lib

d1cc832

To use the latest redeption features.

Update @orangekit/sdk dependency

03ced5b

Add BitcoinRedeemer interface and Ethereum impl

c9cb89b

We need the address of the `BitcoinRedeemer` contract to initialize the withdrawal.

Add signMessage fn in BitcoinProvider

355803e

We need to sign the Bitcoin message in withdrawal flow.

Update stBTC contract inteface

6dceb03

Add `getChainIdentifier` and `encodeApproveAndCallFunctionData` methods. We need the stBTC contract address and encoded function data to initialize the withdrawal.

Implement custom RedeemerProxy

0ec8c7e

The `OrangeKitTbtcRedeemerProxy` requests redemption using the orangekit SDK. It builds necessary data for the orangekit safe transaction and sends transaction using the orangeKit.

Add function to initialize the withdrawal

41029ef

Update BitcoinProvider interface

65da0aa

Add `getPublicKey` method - we need the bitcoin account public key to interact with the orangekit.

r-czajkowski added 🎨 dapp dApp 🔌 sdk TypeScript SDK Library labels Jun 14, 2024

r-czajkowski requested review from nkuba and kkosiorowska June 14, 2024 08:34

kkosiorowska force-pushed the orangekit-integration branch from 213fccc to 844a59b Compare June 17, 2024 09:05

r-czajkowski added 6 commits June 17, 2024 18:35

Add unit test of custom Redeemer Proxy

21d847d

Add unit tests for new stBTC methods

6e960f4

Add unit tests for EthereumBitcoinRedeemer

8470c71

Update Tbtc module unit tests

36a2ba1

Add unit test for `initiateRedemption` method.

Update Account module unit tests

7f5ab71

Add unit tests for `initializeWithdrawal` function.

Fix tbtc-depositor tests

d21fc50

Looks like the `ethers` dependency has been upgraded to `6.13.0` and the interface has changed.

Base automatically changed from orangekit-integration to main June 19, 2024 07:37

r-czajkowski added 3 commits June 19, 2024 11:03

Use exact version of ethers lib

85b6855

Use `v6.10.0` in `solidity` and `sdk` workspaces. The pnpm resolves the `ethers` in SDK to the version used in `solidity` workspace.

Fix linting issues

c39927d

Update Acre.initialize fn

712ee1c

We need the gelato API to create the orangekit SDK.

nkuba reviewed Jun 19, 2024

View reviewed changes

nkuba mentioned this pull request Jun 19, 2024

WIP: Withdrawals #456

Closed

r-czajkowski added 4 commits June 19, 2024 11:32

Merge branch 'main' into withdrawals

0319357

Update docs in stBTC contract handle

f9f5880

Add missing docs in SDK

6e39f0a

Update OrangeKitTbtcRedeemerProxy impl

acc713a

Remove unnecessary `init` fn - use constructor instead.

r-czajkowski marked this pull request as ready for review June 20, 2024 12:00

nkuba mentioned this pull request Jun 20, 2024

Replace BitcoinProvider with import from @orangekit/sdk #489

Open

nkuba reviewed Jun 20, 2024

View reviewed changes

r-czajkowski added 6 commits June 21, 2024 11:24

Update initializeWithdrawal fn

e6e63b0

We want to know when the user needs to sign the message and when he signed it, so here we add callbacks that are triggered before and after the signing process.

Disable some eslint rules for entire file

06620ee

To not repeat them.

Bump the @orangekit/sdk to the latest version

b211e5c

Update useInitializeAcreSdk hook

1359092

Hardcode he Gelato api key in `AcreSdkContext` for consistency.

Merge branch 'main' into withdrawals

0fe6a5d

r-czajkowski requested a review from nkuba June 21, 2024 10:03

kkosiorowska requested changes Jun 21, 2024

View reviewed changes

nkuba mentioned this pull request Jun 24, 2024

Withdraw with Bitcoin signature #353

Closed

3 tasks

r-czajkowski added 2 commits June 24, 2024 11:07

Move hook to the sdk dir

3fd479e

Code clean-up

caa8b39

Use const variable instead of hardcoded value.

r-czajkowski and others added 2 commits June 24, 2024 15:37

Add temporary Error modal for unstaking flow

278e650

Merge branch 'main' into withdrawals

7831b17

r-czajkowski requested a review from kkosiorowska June 24, 2024 13:38

nkuba mentioned this pull request Jun 24, 2024

Calculate withdrawal fees in SDK #495

Merged

Display resume modal correctly in withdrawal flow

807c0cf

We should display the resume modal when the user rejects the sign message step. Otherwsie we want to display the error modal.

kkosiorowska previously approved these changes Jun 26, 2024

View reviewed changes

Merge branch 'main' into withdrawals

53fdad6

r-czajkowski dismissed kkosiorowska’s stale review via 53fdad6 June 26, 2024 11:45

kkosiorowska approved these changes Jun 28, 2024

View reviewed changes

kkosiorowska enabled auto-merge June 28, 2024 07:38

Merge branch 'main' into withdrawals

2421e74

kkosiorowska merged commit 8ae1ec6 into main Jun 28, 2024
24 checks passed

kkosiorowska deleted the withdrawals branch June 28, 2024 07:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Withdrawals #474

Withdrawals #474

r-czajkowski commented Jun 14, 2024 •

edited by nkuba

Loading

nkuba Jun 19, 2024

nkuba Jun 19, 2024

r-czajkowski Jun 19, 2024

nkuba Jun 20, 2024

nkuba Jun 19, 2024

r-czajkowski Jun 19, 2024

nkuba Jun 19, 2024

r-czajkowski Jun 19, 2024

nkuba Jun 19, 2024

r-czajkowski Jun 19, 2024

nkuba Jun 19, 2024

r-czajkowski Jun 19, 2024

nkuba Jun 19, 2024

r-czajkowski Jun 19, 2024

nkuba Jun 20, 2024

kkosiorowska left a comment

r-czajkowski commented Jun 24, 2024 •

edited

Loading

r-czajkowski commented Jun 24, 2024 •

edited

Loading

kkosiorowska left a comment

kkosiorowska left a comment

		// TODO: Should we hide this API key in SDK impl or should it be passed by the
		// consumer?

		@@ -14,4 +14,8 @@ export interface BitcoinProvider {
		* @returns Bitcoin address selected by the user.

	previewRedeem(amount: bigint): Promise<bigint>
	previewRedeem(shares: bigint): Promise<bigint>

Withdrawals #474

Withdrawals #474

Conversation

r-czajkowski commented Jun 14, 2024 • edited by nkuba 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

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

Choose a reason for hiding this comment

kkosiorowska left a comment

Choose a reason for hiding this comment

r-czajkowski commented Jun 24, 2024 • edited Loading

r-czajkowski commented Jun 24, 2024 • edited Loading

kkosiorowska left a comment

Choose a reason for hiding this comment

kkosiorowska left a comment

Choose a reason for hiding this comment

r-czajkowski commented Jun 14, 2024 •

edited by nkuba

Loading

r-czajkowski commented Jun 24, 2024 •

edited

Loading

r-czajkowski commented Jun 24, 2024 •

edited

Loading