Deposit history #427
Deposit history #427
Conversation
r-czajkowski
force-pushed
the
deposit-history
branch
from
ce5b260 to
bad47e9
Compare
sdk/src/lib/api/TbtcApi.ts
Outdated
| return responseData.map((deposit) => ({ | ||
| ...deposit, | ||
| initialAmount: BigInt(deposit.initialAmount), | ||
| depositKey: ethers.solidityPackedKeccak256( |
There was a problem hiding this comment.
Shouldn't we override it only if it's empty? I know the Deposit type returned by the API doesn't contain the depositKey property. Maybe we shouldn't define a generic Deposit type but return the same set of properties the API returns and later in src/modules/tbtc/Tbtc.ts we would calculate the depositKey and convert the initial amount type?
sdk/src/modules/staking/index.ts
Outdated
| ) | ||
|
|
||
| const amount = toSatoshi( | ||
| depositFromSubgraph?.amount ?? deposit.initialAmount, |
There was a problem hiding this comment.
As per conversation let's use the initial amount, regardless of the deposit state.
There was a problem hiding this comment.
So it looks like we do not need the subgraph integration. At least for now, right?
There was a problem hiding this comment.
We don't need it for now, but since you already implemented it maybe it would be worth keeping it. It gives us the possibility to fetch deposits from the chain, in case they are missed in the tBTC API for any reason (e.g. they were created not using the tBTC API).
There was a problem hiding this comment.
(e.g. they were created not using the tBTC API).
So IMO in that case we should get the amount from subgraph otherwise it will be null. I will update getDepositsByOwner from AcreSubgraphApi and add all the "amount" fields, and here we will always use initialAmount but from different sources.
sdk/src/modules/tbtc/Tbtc.ts
Outdated
| @@ -11,7 +11,7 @@ import { IEthereumSignerCompatibleWithEthersV5 as EthereumSignerCompatibleWithEt | |||
| * Represents the tBTC module. | |||
| */ | |||
| export default class Tbtc { | |||
| readonly #tbtcApi: TbtcApi | |||
| public readonly tbtcApi: TbtcApi | |||
There was a problem hiding this comment.
Instead of exposing this property could we define a getDepositsByOwner class that would fetch the data from tBTC API and convert the types?
We are going to use the `tbtcApi` in other modules.
And expose the `getDepositsByOwner` function that returns all deposits for a given depositor.
Expose the `getDepositsByOwner` function that returns all deposits for a given deposit. The main difference between this function and from `AcreSubgraphApi` class is that tbtc api stores queued deposits - this means they have not yet been initialized or finalized. Subgraph only indexes initialized or finalized deposits.
Add function that returns all deposits - `queued`, `initialized` and `finalized` for a given depositor. Depositor address is created besed on the bitcoin address returned by `BitcoinProvider`.
Add Acre subgraph api - used to fetch deposits data.
To fetch deposit activities for a given user.
Return amount in satoshi precision.
Remove "sign message first" requirement. We want to skip the signing message step since there is a bug with this feature in Ledger Live Wallet API.
Insted of exposing the `tbtcApi` property we define `getDepositsByOwner` function that fetches the data from tBTC API and converts types.
r-czajkowski
force-pushed
the
deposit-history
branch
from
300dd16 to
8bf01e0
Compare
From `TbtcApi` class.
Add unit tests for `getDepositsByOwner` method.
Also here we update the `getDepositsByOwner` from `AcreSubgraphApi` - add the `amountToDeposit` and `initialAmount` fields and we should use the initial amount regardless of the deposit state.
| /** | ||
| * Represents the deposit data. | ||
| */ | ||
| export type Deposit = { |
There was a problem hiding this comment.
As far as I can see, we define the Deposit type 3 times. We export only one type but I would consider changing the names of these types a bit. Because there may be confusion when we want to export some more types. Additionally, we mix these types a little with each other. See below.
The getDeposits function returns a Deposit type just like this.#acreSubgraphApi.getDepositsByOwner. But these are different types. This is not a blocking comment, just flagging.
There was a problem hiding this comment.
Since we export the Deposit type just once it shouldn't be too confusing externally. But I agree we need to sort this out sooner rather than later. We plan to refactor the SDK to reduce confusion and simplify usage and development.
We need to return the `timestamp` (when the deposit was created) - we display the time for the activity in the transaction table.
To avoid code duplication and reuse it in unit tests.
We moved the subgraph integration to the SDK so the `VITE_ACRE_SUBGRAPH_URL` is no longer needed.
Make the `type` field required and update the `status` field to `pending` | `completed`. The activity is also a withdrawal so statuses must be generic and the `DepositStatus` type was too specific and it won't fit for the withdrawals. Here we also set correct values in the `useFetchDeposits` hook.
We changed the `status` type from `DepositStatus` to `pending | completed`. Here we use correct equality to check whether the deposit has completed status.
This was accidentally removed during the merge.
✅ Deploy Preview for acre-dapp-testnet ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Depends on: #371
This PR adds support for fetching the deposits in SDK. We must fetch data from both sources tbtc API and Acre subgraph because the tbtc API includes additional
queueddeposits - this means they have not yet been initialized or finalized. The subgraph only indexes initialized and finalized deposits, so we need to combine them.