-
Notifications
You must be signed in to change notification settings - Fork 2
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
Wrap ETH address handling iniside sdk #445
Conversation
Implement the `toAcreIdentifier` function that returns the deposit owner/user identifier in the Acre protocol based on the bitcoin address. The function saves calculated identifier in cache so we do not need to recalculate it.
And update the `sharesBalance` and `estimatedBitcoinBalance` function to rely on the bitcoin address only. The consumer should operate with a Bitcoin address only. SDK should handle conversion from Bitcoin to EVM identifier behind the scenes.
✅ Deploy Preview for acre-dapp-testnet ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
We no longer need to pass the chain identifier to the `sharesBalance` and `estimatedBitcoinBalance` - the SDK relies on the bitcoin provider implementation passed during the SDK initialization.
Do not export the `ethereum` lib components from the root `index.ts` file - we should keep the Ethereum-related stuff under the hood inside the SDK.
The `EthereumNetwork` is no longer available in the `@acre/sdk` package. The dapp should rely on the Bitcoin network only.
In the `Account` module we want to expose the features related to a given bitcoin account.
Require the bitcoin address and acre identifier in consructor. We can calculate the acre identifier only once in `Acre.initialize` function and pass it to the module.
Extract general protocol functions to a separate module. This module should provide general functions related to the Acre protocol such as: fees, min deposit/withdrawal amount, tvl etc.
Sine we resolve the addresses just once on the SDK initialization we can remove this component.
Since some of the dirs contain only `index.ts` file we move them one level higher with name of the module.
Add info about the precision of the returned value.
We should be consistent so we want to return values related to the Bitcoin in satoshi precision (1e8).
We renamed the method `minDepositAmount` to `minimumDepositAmount` and here we update the dapp part.
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.
We should add `0x` prefix to the address when creating the `VoidSigner` instance.
Fixed in db5fdde. |
) | ||
|
||
const signer = new VoidSigner(depositOwnerEvmAddress, ethersProvider) | ||
const signer = new VoidSigner( | ||
`0x${accountEthereumAddress.identifierHex}`, |
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.
Looking at the complexities ChainIdentifier
and EthereumAddress
types introduce (e.g. the issue in #445 (review)) I'm thinking about using plain string
to deal with addresses and convert it to EthereumAddress
only there where tBTC SDK requires it.
WDYT?
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.
It is not blocking this PR. We can handle that separately.
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.
Yeah but that's true. But I think we can add an alias for the EthereumAddress eg. type EthereumAddress = 0x${string}
or type EthereumAddress = string
IMO it improves the readability. WDYT? Anyway, I think we should address it in a separate PR because it requires a lot of changes.
Here we expose in the SDK the totalAssets function from stBTC contract. The function returns total value of tBTC under Acre protocol management.
This reverts commit 0493060. It was pushed by accident to the wrong branch.
Depends on #445 In this PR we expose in the SDK the totalAssets function from stBTC contract. The function returns the total value of tBTC under Acre protocol management.
Closes: #426
Closes: #428
This PR wraps the ETH address handling inside the SDK and refactores some modules.
Here we remove the
Staking
module and split it into 2 separate modulesAccount
andProtocol
. TheAccount
module exposes features related to a given Bitcoin account such as initializing deposits, fetching balance etc. TheProtocol
module provides general information related to the Acre protocol such as fees, minimum deposit/withdrawal amount, tvl etc.