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

Round managment + Update bitvm dependency #572

Merged
merged 58 commits into from
Mar 3, 2025
Merged

Conversation

ekrembal
Copy link
Member

@ekrembal ekrembal commented Feb 28, 2025

Description

Add 2 internal grpc endpoints for operator: finalized payout tx and end round
Add tx names and details in tx_sender for better logging
Handled tx_sender errors, when it panics it stops the whole process
Updated BitVM dependency to not use patches

Linked Issues

Testing

Tested that the operator can get a reimbursment after calling end-round

ekrembal and others added 30 commits February 21, 2025 15:56
- Introduced new database tables for tracking round index and used kickoff connectors
- Implemented `handle_finalized_payout` method in Operator to manage deposit payouts
- Updated transaction signing to return full Transaction instead of RawSignedTx
- Added methods to get and update current round index
- Modified RPC serialization to use bitcoin consensus serialization
@ekrembal ekrembal requested review from mmtftr and atacann March 3, 2025 10:46
@ekrembal ekrembal changed the base branch from ekrem/round-managment-2 to dev March 3, 2025 10:46
@ekrembal ekrembal marked this pull request as ready for review March 3, 2025 10:47
@ekrembal ekrembal requested a review from ceyhunsen March 3, 2025 10:48
disprove_timeout_timelock: 250,
assert_timeout_timelock: 200,
operator_reimburse_timelock: 48,
watchtower_challenge_timeout_timelock: 200,
Copy link

Choose a reason for hiding this comment

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

revert these to use constants

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in 73dcf78

vec![
(self.latest_blockhash_pk.to_vec(), 20),
(self.challenge_sending_watchtowers_pk.to_vec(), 20),
(self.bitvm_pks.0[0].to_vec(), 32),
Copy link

Choose a reason for hiding this comment

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

needs to be * 2 for all WinternitzCommits in this function, because it needs message (digit) length not byte length

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in 6c51b02

// let kp = bitcoin::secp256k1::Keypair::new(&SECP, &mut rand::thread_rng());
// let xonly_pk = kp.public_key().x_only_public_key().0;

// let derivation = WinternitzDerivationPath::BitvmAssert(
Copy link

Choose a reason for hiding this comment

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

Uncomment test, something like below works.
One other idea is that we can only store pk_type and idx here, have a function that returns byte_size of a pk_type inside the new bitvm struct, then we can get message lengths from pk_type so that we do not pass message length everywhere and its easy to change

Suggested change
// let derivation = WinternitzDerivationPath::BitvmAssert(
let derivation = WinternitzDerivationPath::BitvmAssert(
64,
3,
0,
Txid::all_zeros(),
ProtocolParamsetName::Regtest.into(),
);

Copy link
Member Author

Choose a reason for hiding this comment

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

re-added the test in 6781fae

i think it is okay to send the message length for now?

@ekrembal ekrembal changed the title Update bitvm dependency Round managment + Update bitvm dependency Mar 3, 2025
@ekrembal ekrembal merged commit 92f8812 into dev Mar 3, 2025
9 checks passed
@ekrembal ekrembal deleted the ekrem/update-bitvm-2 branch March 3, 2025 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants