Move withdrawal code to grpc (payout tx creator) and enable withdrawal tests #505

ceyhunsen · 2025-02-06T09:53:17Z

Description

Uncomments operator withdrawal code
Implements operator::new_withdrawal_sig rpc call
Implements payout tx creator

Extras

Creates clients in create_actor and returns them instead
Uncomments tests::run_single_deposit
Fixes compilation errors in rcp::honest_operator_takes_refund test

Linked Issues

Closes Operator payout tx for Withdrawal #467

…pilation errors.

…_with_thread_name macro.

atacann

LGTM

core/src/builder/script.rs

…uilder::script.

core/src/rpc/operator.rs

mmtftr · 2025-02-07T13:57:40Z

core/src/test_utils.rs

+            .add_input(
+                NormalSignatureKind::NotStored,
+                txin,
+                SpendPath::Unknown,


this will fail when signing

Should be fixed in ade8477.

…esn't include users_intent_outpoint.

…hdrawal_transaction_and_signature macro.

core/src/rpc/parser/operator.rs

core/src/builder/transaction/operator_reimburse.rs

mmtftr · 2025-02-08T08:50:00Z

core/src/operator.rs

-    //         &user_xonly_pk,
-    //     )?;
+            let txid_response = &response[2..66];
+            let txid = hex::decode(txid_response).map_err(|e| BridgeError::Error(e.to_string()))?;


What does this do exactly? can we add some comments? Referring to a gist with function nmes is not clear enough

This is an old code that copied as is. Can be documented in this PR, though.

Looking at it again, this was about Citrea and not about us. There was a single reference, which is that gist link. Relevant issue: #314

thanks a lot for finding that issue, can we write it into the code as a comment? I don't think we need a fully-fledged crate, but a documented function that calls to Citrea and handles errors properly is definitely needed here. Maybe we could get some help from the Citrea team?

I misused crate keyword there, meant module. But looks like we can get away with function(s) somewhere like in utils module. When we decide to do this, we might need more help from Citrea team (gist is from them). Currently, we don't even utilize this code in tests because we don't have any connection to a Citrea instance, in tests. When we add Clementine support for https://github.com/chainwayxyz/citrea-e2e, we can do that.

Comment added in 8a22a4b

mmtftr

I'm OK with a merge right now, the comments are nitpicks and can be ignored

…ate_payout_txhandler.

core/src/builder/transaction/operator_reimburse.rs

…gned variants.

…anchor output.

mmtftr

LGTM

ceyhunsen added 4 commits February 6, 2025 12:51

refactor: Uncomment new_withdrawal_sig in operator module and fix com…

f6c5318

…pilation errors.

feat: Add payout tx creator in builder.

93365a8

feat: Add witness element in create_payout_txhandler input.

fef363c

feat: Add witness element in create_payout_txhandler input.

9f8c023

ceyhunsen changed the title ~~Move withdrawal code to grpc~~ Move withdrawal code to grpc (payout tx creator) and enable withdrawal tests Feb 6, 2025

ceyhunsen added 7 commits February 6, 2025 17:13

feat: Implement operator::new_withdrawal_sig call.

9f1ed8e

chore: Re-enable rpc::run_single_deposit common function.

11f2993

test: Add client creation for create_actors and return them.

a3a97f1

test: Don't unwrap and return err in run_single_deposit.

f4a1d89

test: Enable honest_operator_takes_refund test.

7633d38

chore: Remove required std::{env, thread} use from create_test_config…

c05c5ce

…_with_thread_name macro.

chore: Remove unused code.

95fc4e2

ceyhunsen force-pushed the ceyhun/withdrawal_with_grpc branch from 0d139d4 to 95fc4e2 Compare February 6, 2025 20:00

ceyhunsen added 3 commits February 7, 2025 10:18

test: Disable honest_operator_takes_refund test in rpc test.

bfc207d

feat: Re-add WithdrawalScript to script builder.

31bc460

Merge branch 'dev' into ceyhun/withdrawal_with_grpc

0aa16d0

ceyhunsen requested review from ekrembal, mmtftr, atacann and ozankaymak February 7, 2025 07:37

ceyhunsen marked this pull request as ready for review February 7, 2025 07:37

chore: Fix compilation errors caused by the merge.

0643836

atacann reviewed Feb 7, 2025

View reviewed changes

core/src/builder/script.rs Outdated Show resolved Hide resolved

ceyhunsen added 3 commits February 7, 2025 14:37

chore: Remove unnecessary generate_witness from WithdrawalScript in b…

d5914f2

…uilder::script.

Merge branch 'dev' into ceyhun/withdrawal_with_grpc

edee58a

chore: Fix compilation errors caused by the merge.

bcc12bd

mmtftr requested changes Feb 7, 2025

View reviewed changes

ceyhunsen added 2 commits February 7, 2025 17:39

fix: Return error if operator::new_withdrawal_sig rpc call's input do…

fee4f72

…esn't include users_intent_outpoint.

test: Add key-spend spend path to withdrawal tx input in generate_wit…

ade8477

…hdrawal_transaction_and_signature macro.

ceyhunsen requested a review from mmtftr February 8, 2025 08:22

mmtftr reviewed Feb 8, 2025

View reviewed changes

core/src/rpc/parser/operator.rs Outdated Show resolved Hide resolved

chore: Convert if let else to a one liner.

ff53e32

mmtftr reviewed Feb 8, 2025

View reviewed changes

core/src/builder/transaction/operator_reimburse.rs Outdated Show resolved Hide resolved

mmtftr reviewed Feb 8, 2025

View reviewed changes

mmtftr approved these changes Feb 8, 2025

View reviewed changes

chore: Convert UnspentTxOut::new to UnspentTxOut::from_partial in cre…

df5a4be

…ate_payout_txhandler.

ceyhunsen force-pushed the ceyhun/withdrawal_with_grpc branch from 018a13f to df5a4be Compare February 8, 2025 09:23

ekrembal requested changes Feb 8, 2025

View reviewed changes

core/src/builder/transaction/operator_reimburse.rs Outdated Show resolved Hide resolved

ceyhunsen added 2 commits February 8, 2025 15:11

fix: Make some txhandler functions available for both signed and unsi…

b24856d

…gned variants.

fix: Return a signed txhandler in create_payout_txhandler and remove …

ccdf5f0

…anchor output.

ceyhunsen requested review from ekrembal and mmtftr February 8, 2025 12:16

docs: Add comment about a Citrea request.

8a22a4b

mmtftr approved these changes Feb 8, 2025

View reviewed changes

ceyhunsen self-assigned this Feb 8, 2025

ekrembal approved these changes Feb 8, 2025

View reviewed changes

ceyhunsen merged commit aa0864a into dev Feb 8, 2025
9 checks passed

ceyhunsen deleted the ceyhun/withdrawal_with_grpc branch February 8, 2025 13:39

ceyhunsen mentioned this pull request Feb 8, 2025

Operator payout tx for Withdrawal #467

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move withdrawal code to grpc (payout tx creator) and enable withdrawal tests #505

Move withdrawal code to grpc (payout tx creator) and enable withdrawal tests #505

ceyhunsen commented Feb 6, 2025 •

edited

Loading

atacann left a comment

mmtftr Feb 7, 2025

ceyhunsen Feb 8, 2025

mmtftr Feb 8, 2025

ceyhunsen Feb 8, 2025

ceyhunsen Feb 8, 2025

mmtftr Feb 8, 2025

ceyhunsen Feb 8, 2025

mmtftr left a comment

mmtftr left a comment

Move withdrawal code to grpc (payout tx creator) and enable withdrawal tests #505

Move withdrawal code to grpc (payout tx creator) and enable withdrawal tests #505

Conversation

ceyhunsen commented Feb 6, 2025 • edited Loading

Description

Extras

Linked Issues

atacann left a comment

Choose a reason for hiding this comment

mmtftr Feb 7, 2025

Choose a reason for hiding this comment

ceyhunsen Feb 8, 2025

Choose a reason for hiding this comment

mmtftr Feb 8, 2025

Choose a reason for hiding this comment

ceyhunsen Feb 8, 2025

Choose a reason for hiding this comment

ceyhunsen Feb 8, 2025

Choose a reason for hiding this comment

mmtftr Feb 8, 2025

Choose a reason for hiding this comment

ceyhunsen Feb 8, 2025

Choose a reason for hiding this comment

mmtftr left a comment

Choose a reason for hiding this comment

mmtftr left a comment

Choose a reason for hiding this comment

ceyhunsen commented Feb 6, 2025 •

edited

Loading