-
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
Acre SDK - staking module #106
Conversation
} | ||
|
||
const types: Types = { | ||
Stake: [{ name: "receiver", type: "address" }], |
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.
Let's also add the Bitcoin refund account property.
And also leave a TODO note to revisit the message structure when working on unstaking messages.
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.
const { depositor: _, ...restDepositReceipt } = this.#deposit.getReceipt() | ||
|
||
const revealDepositInfo = { | ||
fundingOutputIndex: outputIndex, | ||
...restDepositReceipt, | ||
} |
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.
Could you please explain what are we doing here with the depositor?
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 want to get all deposit receipt fields w/o depositor. Since depositor is unused variable the eslint throws an error and to fix it we need to rename it _
.
sdk/test/lib/ethereum/eip712.test.ts
Outdated
EthereumEIP712Signer, | ||
EthereumAddress, | ||
} from "../../../src/lib/ethereum" | ||
import { EthereumSignedMessage } from "../../../src/lib/ethereum/eip712-signer/signed-message" |
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.
Can we use an absolute path in files that have such nesting?
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.
What do you mean?
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.
I'm just thinking about using absolute path, eg: "src/lib" instead of relative paths like "../../../src"
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.
Hmm I can't do that, eslint throws an error Unable to resolve path to module 'src'
. I can dig into it in spare cycles but now I improved imports so the paths are much simpler see 023a33d.
sdk/src/index.ts
Outdated
_tbtc: TBTC, | ||
) { | ||
this.contracts = _contracts | ||
this.#tbtc = _tbtc |
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.
Just for confirmation of the naming convention, do we use the _
prefix for private class members and #
for read-only/protected?
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.
#
is a hard private field in js/ts see https://www.typescriptlang.org/docs/handbook/2/classes.html#caveats.
I use _
in constructor just to indicate that it is a constructor parameter.
c510412
to
2675078
Compare
956d897
to
4d41223
Compare
Use `TbtcDeposit` alias for `Deposit` import and rename the field in the `StakeInitialization` class to `tbtcDepositor`. To indicate this is deposit from tbtc-v2.ts SDK.
The contract name is `TbtcDepositor` not `TBTCDepositor`.
The message should contain the Bitcoin recovery address instead of the public key hash of this address.
The latest version of the `tbtc-v2.ts` lib adds support for revealing deposit with extra data. We need this feature to stake BTC(tBTC) on user's behalf.
The `TBTCDepositor` should implement the `DepositorProxy` interface to reveal the deposit with extra data on user's behalf. Here we also add `encodeExtraData` and `decodeExtraData` functions to encodes and decodes extra data passed to the tBTC Bridge contract. The Acre extra data is staker address and referral number.
Adujst staking module to the new TBTCDepositor interface and the latest version of the `tbtc-v2.ts` lib. We need to initiate the deposit with extra data using the `tbtc-v2.ts` SDK by injecting our implementation of the `DepositorProxy`.
Adjust unit tests to the latest version of tbtc-v2.ts lib that adds support for revealing deposits with extra data via depositor proxy.
We can get the referral and staker from tBTC deposit object.
Expose shared components, feature modules and the `Acre` entry point class from the root `index.ts` file. The `Acre` entry point class currently provides static function to initialize the Acre SDK for Ethereum network.
Pass the `bitcoinRecoveryAddress` to the `StakeInitialization` class. Since we can't easily determine if we should use `true` or `false` in `BitcoinAddressConverter.publicKeyHashToAddress` to convert public key hash to `P2PKH` or `P2WPKH` we can use what the input was w/o doing a conversion. Here we also get rid of: - the bitcoin client from the `StakeInitailization` class because at least for now it is unnecessary, - `referral` field - it is decoded and passed to the contract function under the hood.
We need `build` and `typechain` artifacts from `core` package in `sdk` because we import artifacts and types generated by `typechain` in `sdk` package. To achieve this, we extract a reusable workflow that builds the `core` package and uploads necessary artifacts. Then we use this reusable workflow to run successfully the `sdk` jobs. Here we also update the `jest` config - we should ignore tests inside `dist` dir if exists.
We want to omit the `signerOrProvider` because it points to ethers v5. We use ethers v6 in Acre SDK.
Ignore `sdk` monorepo package - `sdk` package has own prettier config.
According to the contract source code, the referral should be `uint16`. Here we also add more unit tests to cover more cases eg. when a referral is `0` (min `uint16`) and `65535` (max `uint16`).
We do not expect to support it.
And create the ethereum contract instance based on this artifact.
`receiver` -> `ethereumStakerAddress`. Add more context to the name of field for clarity when user sees it on the device, so they know what the value means.
Check if the staker address is a valid ethereum and non-zero address.
8cc8ca3
to
faa487c
Compare
Pass the `staker` value the same as was used in this function input parameter and use it insied `StakeInitialization` constructor to initialize the value of `#staker` field, instead of recovering it from the ` _tbtcDeposit.getReceipt()` extra data, as this is an external library that can be compromised and return invalid value.
Depends on: #106 Added getting of tbtc deposit receipt for StakeInitialization.
We decided to rename receiver to staker in the Depositor contract and SDK. See: #106 (comment)
Closes #147
Depends on: #91
This PR adds staking module to the Acre SDK. The Acre SDK is built on top of the tbtc-v2.ts lib and exposes interfaces that should be implemented for a given chain to communicate with Acre network. In this PR we add Ethereum implementation and developers can initialize the SDK for the Ethereum chain.
What has been done:
TBTCDepositor
,ChainEIP712Signer
andChainSignedMessage
,tbtc-v2.ts
lib that are used in SDK and are exposed outside the SDK egHex
,ChainIdentifier
,EthereumAddress
andBitcoinRawTxVectors
.The SDK file structure
lib
- shared library componentsContains all components used to build Acre features eg. staking, inspired by
tbtc-v2.ts
lib.bitcoin
: Contains all interfaces to interact with Bitcoin chain,contracts
: Includes abstract interfaces to interact with Acre smart contracts. Should be implemented for a given chain eg. Ethereum.eip712-signer
: An abstract layer for typed structured data signer. The main idea is to hide the signing process under the exact implementation for a given chain and not expose the external library outside the SDK. For example, we can useethers
lib for Ethereum implementation, but different lib for other chains.ethereum
: Provides Ethereum implementation of Acre smart contract and signer interfaces. Should contain other implementation of components that should depend on the chain implementation.utils
: General utility functions.modules
- Acre network featuresProvides access to the Acre network features such as staking.
staking
- provides access to the staking flow for BTC stakers.Staking module
initializeStake
Initializes the Acre staking flow and returns
StakeInitialization
object that exposes staking flow steps:getBitcoinAddress
- returns Bitcoin address corresponding to this deposit. The user should send BTC to this address to stake tokens.signMessage
- signs the staking message and stores it in object instance to use it instake
function. The signed message is required by Acre staking flow.stake
- Stakes BTC based on the Bitcoin funding transaction viaTBTCDepositor
contract. It requires signed staking message, which meansstake
should be called after message signing. By default, it detects and uses the outpoint of the recent Bitcoin funding transaction and throws if such a transaction does not exist. The staking message must be signed first otherwise throws an error.Unit tests
Decided to switch from
mocha
andchain
tojest
.Jest
testing framework has built-in support for mocks, stubs, and spies. Chai requires the installation of additional dependencies but most of them are no longer maintained. I also tried using thechai
withethereum-waffle
package, which adds support for mocking contracts, but it only supports ethers in v5, we want to use the latest version of ethers - it is v6.