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.

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

Explore and develop an alternative for the wallet coordinator mechanism #737

Closed
10 tasks done
lukasz-zimnoch opened this issue Nov 6, 2023 · 0 comments
Closed
10 tasks done
Assignees
Labels
⛓️ solidity Solidity contracts

Comments

@lukasz-zimnoch
Copy link
Member

lukasz-zimnoch commented Nov 6, 2023

The current WalletCoordinator contract was an initial attempt to develop a wallet coordination mechanism without implementing a consensus mechanism in the off-chain client. This mechanism has been working well so far. However, it has a major drawback: it is a single point of failure as it relies on a small set of coordinator bots authorized to submit wallet action proposals. We need to replace it with something better.

This task is about exploring possible alternatives and implementing the most promising ones.

Tasks

  1. 📖 documentation
    lukasz-zimnoch
  2. 📟 client
    lukasz-zimnoch
  3. 📟 client
    lukasz-zimnoch
  4. 📟 client
    lukasz-zimnoch
  5. 📟 client
    lukasz-zimnoch
  6. ⛓️ solidity
    lukasz-zimnoch
  7. 📟 client
    lukasz-zimnoch
  8. 📟 client
    lukasz-zimnoch
  9. 📟 client
    lukasz-zimnoch
@lukasz-zimnoch lukasz-zimnoch self-assigned this Nov 6, 2023
@lukasz-zimnoch lukasz-zimnoch added the ⛓️ solidity Solidity contracts label Nov 6, 2023
tomaszslabon added a commit to keep-network/keep-core that referenced this issue Nov 27, 2023
Refs: keep-network/tbtc-v2#737

Here we present the first part of the changes meant to implement [RFC
12: Decentralized wallet
coordination](https://github.com/keep-network/tbtc-v2/blob/main/docs/rfc/rfc-12.adoc)
in the tBTC wallet client. This pull request focuses on the groundwork
necessary to run the off-chain coordination layer.

### Coordination layer orchestration

The node orchestrates the coordination layer upon startup. Specifically,
it runs two separate goroutines:
- Coordination window watch
- Coordination result processor

Both goroutines are steered by the node's root context and communicate
through a dedicated channel.

### Coordination window watch

The coordination window watch goroutine is responsible for detecting
coordination windows that occur every `900` blocks. Once a window is
detected, it runs the window handler function and passes the window data
as an argument.
The window watch process guarantees that the window handler is called
only once for the given coordination window. Moreover, it also
guarantees that the window handler is called for all coordination
windows. This is achieved by calling each handler in a separate
goroutine so the watch loop does not block for long and the chance of
missing a coordination window signal is negligible.

The window handler function triggers the window processing logic.
Specific steps of that logic are:
1. Determine the list of wallets controlled by the given node
2. Take a coordination executor for each wallet
3. Use the coordination executors to run the coordination procedure for
each wallet (in parallel)
4. Collect coordination results and push them to the processing channel

### Coordination executor

The coordination executor is a component that is responsible for running
the coordination procedure for the given wallet and coordination window.
It is designed to encapsulate the logic of the procedure (coordination
seed, communication, and so on). It also ensures that only one instance
of the procedure is executed at a time. The executor is also responsible
for assembling the coordination procedure's result and reporting all
coordination faults detected during execution.

The design of the coordination executor is inspired by the existing
signing and DKG executor. It attempts to fit the coordination
procedure's logic into the existing codebase in an elegant way.

### Coordination result processor

The coordination result processor goroutine listens for incoming
coordination results and triggers the result handler function in a
separate goroutine to ensure all results are processed independently.

The result handler function triggers the result processing logic.
Specific steps of that logic are:
1. Record coordination faults reported in the result
2. Detect the type of the action proposal being part of the result
3. Fire the appropriate proposal handler

The proposal handlers are part of the existing codebase. They are
responsible for the orchestration and execution of the proposed wallet
actions.

### Intersection with the existing chain-based coordination mechanism

The presented groundwork was built alongside the existing chain-based
coordination mechanism. Some initial integration steps around data types
were done. The existing mechanism will be gradually removed and replaced
in the follow-up pull requests.

### Next steps

The next steps (coarse-grained) on the way towards RFC 12 implementation
are:

- Implement coordination procedure logic (i.e. implement the
`coordinationExecutor.coordinate` method)
- Finalize coordination result processing (i.e. implement the
`processCoordinationResult` function and refactor `node`'s handlers
appropriately)
- Remove the existing chain-based mechanism (i.e. detach
`WalletCoordinator`'s events handlers and remove unnecessary code from
`chain.go`)
- Modify the SPV maintainter to not rely on `WalletCoordinator`'s events
during unproven transactions lookup
tomaszslabon added a commit to keep-network/keep-core that referenced this issue Nov 30, 2023
Refs: keep-network/tbtc-v2#737
Depends on: #3744

Here we present the second part of the changes meant to implement [RFC
12: Decentralized wallet
coordination](https://github.com/keep-network/tbtc-v2/blob/main/docs/rfc/rfc-12.adoc)
in the tBTC wallet client. This pull request focuses on the coordination
procedure.

On the code level, this is about implementing the
`coordinationExecutor.coordinate` function.

### Coordination seed

Calculation of the coordination seed was implemented in the
`coordinationExecutor.getSeed` function. This function computes the seed
for the given coordination window according to the RFC 12 specification:
```
coordination_seed = sha256(wallet_public_key_hash | safe_block_hash)
```
The computed value is used to initialize the RNG for random operations
executed as part of the coordination procedure. Those operations are:
- Leader selection
- Heartbeat execution 

### Coordination leader

Leader selection was implemented in the `coordinationExecutor.getLeader`
function. It uses the coordination seed to select the leader operator
from all operators backing the coordinated wallet. The exact algorithm
is:
1. Get the list of unique operators
2. Sort the list by address in the ascending order
3. Randomly shuffle the list using an RNG initialized with the
coordination seed
4. Pick the first operator from the shuffled list

### Actions checklist

The next step is determining which wallet actions should be checked and
possibly proposed during the coordination window. A list of such actions
is called **actions checklist**. Actions on the list are ordered by
priority, in descending order.

For example, if the list is: `[Redemption, DepositSweep, Heartbeat]`,
the leader must start by checking if redemption can be proposed. If so,
it should do it. Otherwise, it should check the deposit sweep. If
neither redemption nor deposit sweep can be proposed, the leader should
propose a heartbeat.

Actions checklist assembling was implemented in the
`coordinationExecutor.getActionsChecklist` function. The exact checklist
depends on the coordination window. The specific algorithm is:
1. Check the possibility of redemption on every window
2. Check the possibility of a deposit sweep, then moved funds sweep,
then moving funds, every 16th window
3. Draw a decision regarding the heartbeat proposal, with a 12.5% chance
of success
 
### Leader's routine

If the given operator finds itself to be a leader for the given
coordination window, it executes the leader's routine implemented in the
`coordinationExecutor.executeLeaderRoutine` function. It uses the
actions checklist to generate a proposal and broadcasts the proposal to
the followers, using the underlying broadcast channel. It completes the
coordination procedure returning the generated proposal as a result.

### Proposal generator

The leader uses the proposal generator function to generate a proposal
based on the actions checklist for the given window. The generator is
expected to return a proposal for the first action from the checklist
that is valid for the given wallet's state. If none of the actions are
valid, the generator returns a no-op proposal.

Implementation of the proposal generator is beyond the scope of this
pull request and will be handled in a follow-up PR. The plan is to use
the code from `pkg/maintainer/wallet` that is currently used by the
wallet maintainer bot.

### Follower's routine

If the given operator is not the leader for the given coordination
window, it executes the follower's routine implemented in the
`coordinationExecutor.executeFollowerRoutine` function. It listens for
incoming coordination messages and accepts the first message that:
- Is of the proper type
- Is not a self-message
- Comes from a sender with a confirmed wallet membership
- Refers to the currently processed coordination window
- Refers to the coordinated wallet
- Comes from the coordination leader designated for the given window
- Holds a proposal referring to an action from the checklist or a no-op
proposal

The operator-follower completes the coordination procedure returning the
received proposal as a result.

### Next steps

The next steps on the way towards RFC 12 implementation are:

- Implement proposal generation
- Finalize coordination result processing (i.e. implement the
`processCoordinationResult` function and refactor `node`'s handlers
appropriately)
- Remove the existing chain-based mechanism (i.e. detach
`WalletCoordinator`'s events handlers and remove unnecessary code from
`chain.go`)
- Modify the SPV maintainter to not rely on `WalletCoordinator`'s events
during unproven transactions lookup
tomaszslabon added a commit to keep-network/keep-core that referenced this issue Dec 4, 2023
Refs: keep-network/tbtc-v2#737
Depends on: #3745

Here we present the third part of the changes meant to implement [RFC
12: Decentralized wallet
coordination](https://github.com/keep-network/tbtc-v2/blob/main/docs/rfc/rfc-12.adoc)
in the tBTC wallet client. This pull request focuses on proposal
generation.

### Remove wallet coordination from the maintainer module

So far, the maintainer bot implemented in the `pkg/maintainer` package
was responsible for wallet coordination. The logic was living in the
`pkg/maintainer/wallet` sub-package. As the maintainer bot is no longer
responsible for wallet coordination, we are detaching the wallet
coordination from there. This has also an impact on the maintainer-cli.
Commands responsible for deposit sweep and redemption proposal
submission are no longer available.

### Move code from `pkg/maintainer/wallet` package to `pkg/tbtcpg`

Although the maintainer no longer uses the wallet coordination code,
that code is still useful for the new coordination mechanism. It
contains the logic necessary to produce coordination proposals. Hence,
we moved it to the new `pkg/tbtcpg` package and exposed an entry point
component `ProposalGenerator` that implements the
`tbtc.CoordinationProposalGenerator` interface. Thanks to that, the
`pkg/tbtc` package can use the code from `pkg/tbtcpg` to generate
coordination proposals.

Ideally, the code from `pkg/tbtcpg` should be embedded into `pkg/tbtc`.
However, both packages are not compatible out of the box. Merging them
would require a lot of breaking changes. As RFC 12 implementation is
already a complex task, we decided to keep `pkg/tbtcpg` as a separate
being for now, to reduce risk.

Last but not least, the code in `pkg/tbtcpg` was simplified. This code
no longer needs to handle proposals for multiple wallets at the same
time so focusing on a single wallet allowed us to remove redundant code
and facilitate further maintenance.

### Wire up `pkg/tbtcpg` package to `pkg/tbtc`

As mentioned in the previous section, the `pkg/tbtcpg` implements the
`tbtc.CoordinationProposalGenerator` interface so it can be used to
generate proposals within the new coordination mechanism. This was
achieved by injecting the `tbtcpg.ProposalGenerator` as a dependency to
`tbtc.node`, during the setup process.

### Next steps

The next steps on the way towards RFC 12 implementation are:

- Finalize coordination result processing (i.e. implement the
`processCoordinationResult` function and refactor `node`'s handlers
appropriately)
- Remove the existing chain-based mechanism (i.e. detach
`WalletCoordinator`'s events handlers and remove unnecessary code from
`chain.go`)
- Modify the SPV maintainter to not rely on `WalletCoordinator`'s events
during unproven transactions lookup
tomaszslabon added a commit to keep-network/keep-core that referenced this issue Dec 5, 2023
Refs: keep-network/tbtc-v2#737
Depends on: #3747

Here we present the fourth part of the changes meant to implement [RFC
12: Decentralized wallet
coordination](https://github.com/keep-network/tbtc-v2/blob/main/docs/rfc/rfc-12.adoc)
in the tBTC wallet client. This pull request focuses on the coordination
result processing.

### Result processor

Here we implement the `processCoordinationResult` function that receives
coordination results, detects the proposal type, and triggers the
appropriate proposal handler exposed by the `node` component. This
function also sets up the proposal processing start block (i.e.
coordination window end block) and the proposal expiry block.

### Detach `WalletCoordinator` event handlers

Due to the above changes, from now on, the `node` component fires wallet
actions based on incoming coordination results. The `node` component no
longer listens for events coming from the `WalletCoordinator` contract.
We are taking the opportunity and remove those handlers along with the
auxiliary code (e.g. events deduplication).

### Next steps

The next steps on the way towards RFC 12 implementation are:

- Write `WalletProposalValidator` contract and integrate it with the
client
- Cleanup `WalletCoordinator` leftovers in the client
- Modify the SPV maintainter to not rely on `WalletCoordinator`'s events
during unproven transactions lookup
tomaszslabon added a commit that referenced this issue Dec 5, 2023
Refs: #737

Here we present the fifth part of the changes meant to implement [RFC
12: Decentralized wallet
coordination](https://github.com/keep-network/tbtc-v2/blob/main/docs/rfc/rfc-12.adoc)
in the tBTC wallet client. This pull request focuses on the on-chain
proposal validation, i.e. the `WalletProposalValidator` contract
described in RFC 12.

### The `WalletProposalValidator` contract

Here we introduce the read-only, non-upgradeable,
`WalletProposalValidator` contract that allows validating wallet action
proposals. This contract exposes three functions:
- `validateDepositSweepProposal` that allows validating deposit sweep
proposals. This function was ported from the `WalletCoordinator`
contract.
- `validateRedemptionProposal` that allows validating redemption
proposals. This function was ported from the `WalletCoordinator`
contract.
- `validateHeartbeatProposal` that allows validating heartbeat
proposals.

### Remove `WalletCoordinator` contract

We are also removing the `WalletCoordinator` contract. It is no longer
needed with the new RFC-12-based coordination mechanism. The relevant
view validation functions were ported to the aforementioned
`WalletProposalValidator` contract.
By the way, we are also adjusting the deployment scripts to reflect
contract changes made in this PR.

### Next steps

The next steps on the way towards RFC 12 implementation are:

- Integrate `WalletProposalValidator` contract with the client and
cleanup `WalletCoordinator` leftovers there
- Modify the SPV maintainer to not rely on `WalletCoordinator`'s events
during unproven transactions lookup
tomaszslabon added a commit to keep-network/keep-core that referenced this issue Dec 7, 2023
…or and final cleanup (#3751)

Refs: keep-network/tbtc-v2#737

Here we present the sixth part of the changes meant to implement [RFC
12: Decentralized wallet
coordination](https://github.com/keep-network/tbtc-v2/blob/main/docs/rfc/rfc-12.adoc)
in the tBTC wallet client. This pull request focuses on integrating with
the new `WalletProposalValidator` contract introduced in
keep-network/tbtc-v2#756. It also aims to clean
up all leftovers related to the old mechanism based on the
`WalletCoordinator` contract.

### Use `WalletProposalValidator` for on-chain proposal validation

We re-generated contract bindings to leverage the new
WalletProposalValidator contract for on-chain proposal validation.
Incoming deposit sweep, redemption, and heartbeat proposals are now
validated against this contract, and we no longer use
`WalletCoordinator` for that purpose.

### Cleanup `WalletCoordinator` leftovers

We completely removed the `WalletCoordinator` contract from the mix. The
client no longer uses it in any way. This pull request's scope includes
all necessary code cleanup and adjustments.

### Adjust SPV maintainer

The SPV maintainer was using recent proposals submitted to the
`WalletCoordinator` contract to determine a list of wallets that must be
checked as part of the SPV proof submission process. As
`WalletCoordinator` is no longer used, we modified that behavior. The
SPV maintainer now examines recent deposit revealed and redemption
requested events to build a list of wallets that likely have unproven
transactions.

### Adjust maintainer deployment manifests

As wallet coordination was removed from the Keep maintainer bot, we
adjusted Kubernetes manifests that are used to deploy the maintainer on
production and test clusters. Now, only SPV and Bitcoin difficulty
modules are in use.
tomaszslabon added a commit to keep-network/keep-core that referenced this issue Dec 7, 2023
Refs: keep-network/tbtc-v2#737

Here we add the missing case that stops coordination result processing
in case a no-op proposal is issued. The no-op proposals cannot be
processed as they lead to panic. By the way, we are fixing some problems
with log displaying discovered during testing.
michalinacienciala added a commit to keep-network/keep-core that referenced this issue Dec 19, 2023
Refs: keep-network/tbtc-v2#737

The currently used probability of 12.5% means that a completely idle
wallet will perform a heartbeat every 4 coordination windows on average
(or 8 in the worst case) so, every 3600 blocks (~12 hours assuming a
coordination every 900 blocks and 12 seconds per Ethereum block).

User acceptance tests executed on our Sepolia testnet (10 live wallets)
show that such a value often leads to simultaneous heartbeats for
several wallets at the same time. This, in turn, can cause increased
resource consumption for individual nodes and harm some signing
processes. Although nothing bad happens if those are just heartbeats,
this may be problematic for redemptions and deposit sweeps. To lower the
risk of signing failures, we are lowering the heartbeat probability to
6.25%.

The probability of 6.25% means that a completely idle wallet will
perform a heartbeat every 8 coordination windows on average (or 16 in
the worst case) so, every 7200 blocks (~24 hours assuming a coordination
every 900 blocks and 12 seconds per Ethereum block).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⛓️ solidity Solidity contracts
Projects
None yet
Development

No branches or pull requests

1 participant