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

STR-734: OPERATOR REASSIGNMENT #557

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 73 additions & 0 deletions functional-tests/fn_bridge_reassignment.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import time

import flexitest

import testenv
from constants import UNSPENDABLE_ADDRESS
from fn_bridge_withdraw_happy import (
DEPOSIT_AMOUNT,
check_initial_eth_balance,
deposit_twice,
do_withdrawal,
setup_services,
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should move these to utils?
What you think?
They are being used elsewhere than fn_bridge_withdraw_happy.



@flexitest.register
class BridgeWithdrawReassignmentTest(testenv.StrataTester):
"""
Makes two DRT deposits, then triggers the withdrawal.
The bridge client associated with assigned operator id is stopped.
After the dispatch assignment duration is over,
Check if new operator is being assigned or not
voidash marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Check if new operator is being assigned or not
Check if new operator is being assigned or not
Ensure that the withdrawal resumes and completes successfully.

"""

def __init__(self, ctx: flexitest.InitContext):
# Example: we spin up 5 operators
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Example: we spin up 5 operators
# Example: we spin up 3 operators

Actually, you can just remove this comment as it is very self-evident.

ctx.set_env(testenv.BasicEnvConfig(101, n_operators=5))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that you can also disable auto-generate BTC blocks. This is important to avoid flaky tests when you need total control of the block production (i.e. waiting for 64 blocks below).


voidash marked this conversation as resolved.
Show resolved Hide resolved
def main(self, ctx: flexitest.RunContext):
address = ctx.env.gen_ext_btc_address()
withdraw_address = ctx.env.gen_ext_btc_address()
self.debug(f"Address: {address}")
self.debug(f"Change Address: {withdraw_address}")

btc, seq, reth, seqrpc, btcrpc, rethrpc, web3, el_address = setup_services(ctx, self.debug)

# Check initial balance is 0
check_initial_eth_balance(rethrpc, el_address, self.debug)

# Perform two deposits
deposit_twice(ctx, web3, seqrpc, rethrpc, el_address, self.debug)
Comment on lines +46 to +50
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that there's a way for you to setup an environment with already balances in BTC and Strata, AFAIK @evgenyzdanovich implemented that feature.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This aligns with the "minimal resources" principle above.


# Perform a withdrawal
l2_tx_hash, tx_receipt, total_gas_used = do_withdrawal(
web3, rethrpc, el_address, withdraw_address, self.debug, DEPOSIT_AMOUNT
)

# Check assigned operator
duties = seqrpc.strata_getBridgeDuties(0, 0)["duties"]
withdraw_duty = [d for d in duties if d["type"] == "FulfillWithdrawal"][0]
assigned_op_idx = withdraw_duty["payload"]["assigned_operator_idx"]
assigned_operator = ctx.get_service(f"bridge.{assigned_op_idx}")
self.debug(f"Assigned operator index: {assigned_op_idx}")

# Stop assigned operator
self.debug("Stopping assigned operator ...")
assigned_operator.stop()

# Let enough blocks pass so the assignment times out
# (64 is the dispatch_assignment_duration)
btcrpc.proxy.generatetoaddress(64, UNSPENDABLE_ADDRESS)
time.sleep(3)

# Re-check duties
duties = seqrpc.strata_getBridgeDuties(0, 0)["duties"]
withdraw_duty = [d for d in duties if d["type"] == "FulfillWithdrawal"][0]
new_assigned_op_idx = withdraw_duty["payload"]["assigned_operator_idx"]
new_assigned_operator = ctx.get_service(f"bridge.{new_assigned_op_idx}")

# Ensure a new operator is assigned
assert new_assigned_operator != assigned_operator, "No new operator was assigned"

return True
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are testing only 2 out of 3 things that are highlighted in the original ticket (STR-734):

  1. Is the user’s withdrawal request confirmed in a Strata batch?
  2. After the first operator fails to initiate the withdrawal, is the withdrawal request properly assigned to another operator?
  3. After the withdrawal request is assigned to another operator, is it signed by all other operators, broadcast, and confirmed on signet?

We need to find a way to test number 3 above.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I guess once we observe that it's been reassigned we can restart the operator we shut down and then get it signed.

Loading
Loading