Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Automate deposit and redemption WalletCoordinator actions #3614

Closed
12 tasks done
pdyraga opened this issue Jun 7, 2023 · 0 comments · Fixed by #3668
Closed
12 tasks done

Automate deposit and redemption WalletCoordinator actions #3614

pdyraga opened this issue Jun 7, 2023 · 0 comments · Fixed by #3668
Assignees
@nkuba @lukasz-zimnoch
Labels
📟 client
Milestone
v2.0.0-m4

Comments

@pdyraga
Copy link
Member

pdyraga commented Jun 7, 2023 •
edited by lukasz-zimnoch
Loading

The WalletCoordinator contract is a synchronization point for the network clients. They observe events emitted from this contract and execute the given action previously performing all the required validations of the proposal.

Deposit sweep proposals are currently submitted manually by the dev team. This is possible because sweeps are not as time-sensitive as redemptions but still, we consider it a temporary measure until the proper automation is implemented.

This issue aims at automating both deposit sweep proposals and redemption proposals in the WalletCoordinator. This code should be a part of the existing maintainer module in the client, activated/deactivated based on the config flag.

The logic should be simple for the start: check every 3 hours if there are any redemptions requested. If yes, check if no sweeping is in progress and request redemption of max N deposits. N should be adjustable in the WalletCoordinator contract and set based on the gas cost of submitting redemption SPV proof for the given N.

For the start, sweeps should be proposed every 48 hours if there are no redemptions pending. Given we have optimistic minting, sweeps are not time-sensitive.

When working on this issue please ensure the approach is consistent with SPV proof maintainer module from #3615.

The solution should integrate with redemptions requests implementation from #3609.

Pull Requests
Preview Give feedback
  1. Automate Deposits Sweeps CLI #3626
    📟 client
    nkuba
  2. Return an error if estimated deposit sweep fee is too high #3666
    📟 client
    lukasz-zimnoch
  3. Move internal test scenarios data closer to the dedicated package #3631
    📟 client
    nkuba
  4. Handle deposits sweep coordination in the maintainer #3635
    📟 client
    nkuba
  5. Propose redemptions coordinator with CLI command #3640
    📟 client
    lukasz-zimnoch nkuba
  6. Estimate redemption transaction fee during proposal #3651
    📟 client
    lukasz-zimnoch
  7. Remove pkg/coordinator package #3659
    📟 client
    lukasz-zimnoch
  8. Invoke automated redemptions periodically by maintainer #3660
    📟 client
    lukasz-zimnoch
  9. Rename coordinator to maintainer-cli #3661
    📟 client
    nkuba
  10. Maintainer optimizations around redemptions #3663
    📟 client
    lukasz-zimnoch
  11. Unit tests for wallet maintainer redemptions #3668
    📟 client
    lukasz-zimnoch
  12. Wallet maintainer: Prevent against requestTimeout being max(uint32) #3681
    📟 client
    lukasz-zimnoch
@pdyraga pdyraga added this to the v2.0.0-m4 milestone Jun 7, 2023
@pdyraga pdyraga mentioned this issue Jun 7, 2023
Closed
This was referenced Jun 9, 2023
Merged
Merged
pdyraga added a commit that referenced this issue Jun 14, 2023
@pdyraga
Automate Deposits Sweeps CLI (#3626)
3e9e643 
The changes here are a step on the road to the fully automated sweeps
(#3614).

We update the `propose-deposits-sweep` command initially implemented in
#3549.

The main change to the command is that if deposits are not provided as
arguments, they will be automatically fetched from the chain.

The command will find the oldest unswept deposits for the oldest unswept
wallet and construct a sweep proposal for them.

Number of deposits for the proposal is based on the
[`depositSweepMaxSize`](https://github.com/keep-network/tbtc-v2/blob/66a04c618e140230d25ea0d784ed75e6ba3030a2/solidity/contracts/bridge/WalletCoordinator.sol#L191)
defined in the `WalletCoordinator` contract. The value can be
overwritten with `--deposit-sweep-max-size <number>`.

If `--wallet <wallet-public-key-hash>` is provided the command will find
unswept deposits for the particular wallet.

If `--fee <number>` is not provided the command will estimate a fee for
the sweep transaction.

## Sample Command
To execute the command in dry-run mode:
```sh
# Sweep deposits from the oldest unswept wallet:
LOG_LEVEL=debug go run . coordinator --testnet propose-deposits-sweep --dry-run

# Sweep deposits from a specific wallet:
LOG_LEVEL=debug go run . coordinator --testnet propose-deposits-sweep --wallet 0x7670343fc00ccc2d0cd65360e6ad400697ea0fed --dry-run
```
pdyraga added a commit that referenced this issue Jun 19, 2023
@pdyraga
Handle deposits sweep coordination in the maintainer (#3635)
79fde24 
This is an initial implementation of the Wallet Coordination Maintainer
module. In this PR we handle Deposits Sweep Proposals.

To run maintainer with this module add `--walletCoordination` flag to
the `maintainer` start command, or execute the command without any
module flags specified.

The maintainer will fetch deposits that need sweeping and propose a
deposit sweep every 48 hours.

If the `WalletCoordinator` is locked the process will wait until the
lock is released and submit the deposit sweep proposal afterward.

Refs: #3614
@nkuba nkuba mentioned this issue Jun 19, 2023
Merged
This was referenced Jun 27, 2023
Merged
Merged
Merged
@nkuba nkuba mentioned this issue Jul 3, 2023
Merged
This was referenced Jul 3, 2023
Merged
Merged
Merged
@lukasz-zimnoch lukasz-zimnoch mentioned this issue Jul 13, 2023
Merged
nkuba added a commit that referenced this issue Jul 13, 2023
@nkuba
Wallet maintainer: Prevent against requestTimeout being `max(uint32…
584f47c 
…)` (#3681)

Refs: #3614

While running the redemption task, the wallet maintainer fetches past
`RedemptionRequested` events using a filter whose start block is
calculated as:
```
startBlock := currentBlock - requestTimeoutBlocks - 1000
```
where `requestTimeoutBlocks` is computed as:
```
requestTimeoutBlocks := uint64(requestTimeout) /
    uint64(chain.AverageBlockTime().Seconds())
```
However, the current `Bridge.requestTimeout` on mainnet is set to
`max(uint32)` as redemptions are not yet enabled. That means the
`requestTimeoutBlocks` is higher than `currentBlock` and produces a
negative `startBlock` which is eventually set to `max(uint64)` due to
being an unsigned type. The `startBlock` is then converted to `int64`
within `go-ethereum` library which produces the `invalid argument 0:
block number larger than int64` error as a result.

In order to fix this problem, we are introducing an additional check
that ensures the minimum possible value of `startBlock` is `0`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nkuba nkuba

@lukasz-zimnoch lukasz-zimnoch

Labels
📟 client
Projects
None yet
Milestone
v2.0.0-m4
Development

Successfully merging a pull request may close this issue.

Unit tests for wallet maintainer redemptions keep-network/keep-core
3 participants
@pdyraga @nkuba @lukasz-zimnoch