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

Merge MuSig2 functions into ekrem/new-architecture #206

Merged
merged 21 commits into from
Aug 9, 2024

Conversation

ozankaymak
Copy link
Contributor

@ozankaymak ozankaymak commented Aug 6, 2024

Description

This PR introduces the complete musig2 functionality, built on top of this
musig2 implementation.

Linked Issues

Related to #205.

Testing

Unit tests, dummy transaction verification tests, and rpc tests are implemented.

Docs

Code is documented.

@ozankaymak ozankaymak changed the base branch from main to ekrem/new-architecture August 6, 2024 13:54
Copy link
Member

@ekrembal ekrembal left a comment

Choose a reason for hiding this comment

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

I think whats left is aggregate nonces and partial signatures. And tests also

core/Cargo.toml Outdated
@@ -28,6 +28,7 @@ clap = { workspace = true, features = ["derive"] }
toml = { workspace = true }
sqlx = { workspace = true, features = ["runtime-tokio", "postgres"] }
bitcoin-mock-rpc = { workspace = true }
musig2 = { version = "0.0.11" }
Copy link
Member

Choose a reason for hiding this comment

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

we can use workspace=true here


impl From<musig2::errors::KeyAggError> for BridgeError {
fn from(_err: musig2::errors::KeyAggError) -> Self {
BridgeError::KeyAggContextError
Copy link
Member

Choose a reason for hiding this comment

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

Pls propogate the original message in this

@ekrembal
Copy link
Member

ekrembal commented Aug 6, 2024

We also need tweaked signatures as well

Copy link
Member

@ekrembal ekrembal left a comment

Choose a reason for hiding this comment

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

Add test for signing taproot txs

// MuSigSecNonce consists of two scalars, so it's 64 bytes.
pub type MuSigSecNonce = [u8; 64];
// MuSigAggNonce is a scalar, so it's 32 bytes.
pub type MuSigAggNonce = [u8; 32];
Copy link
Member

Choose a reason for hiding this comment

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

This should be two curve points

keypair: &secp256k1::Keypair,
rng: &mut impl Rng,
pks: Vec<PublicKey>,
tweak: Option<[u8; 32]>,
Copy link
Member

Choose a reason for hiding this comment

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

we don't need tweak and pks here

@ekrembal
Copy link
Member

ekrembal commented Aug 7, 2024

I think we need 3 tests:

  • one for signing a 32 byte hash and verifying the agg signature with agg pubkey
  • one for signing a taproot key spend that does have some script spending paths so tweak is needed
  • one for signing a taproot from a script spending path with aggPubkey checkSig

@ozankaymak ozankaymak marked this pull request as ready for review August 8, 2024 13:44
return Err(BridgeError::TaprootScriptError);
}

let taproot_builder = if n > 1 {
Copy link
Member

Choose a reason for hiding this comment

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

There is a code duplication here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do not understand what you mean.

@ekrembal ekrembal merged commit 05cba06 into ekrem/new-architecture Aug 9, 2024
@ekrembal ekrembal deleted the ozan/new-architecture-musig2 branch August 9, 2024 12:47
ekrembal added a commit that referenced this pull request Aug 29, 2024
* Set rpc endpoints and impl operator new deposit

* Impl verifier new_deposit

* Impl operator_kickoffs_generated

* impl more verifier functions

* Merge MuSig2 functions into ekrem/new-architecture (#206)

* Add some musig2 functionalities

* Some changes in general and in MuSig2

* Change musig2 functions

* Add one small test

* Added three more simple tests

* Add test with dummy_tx key_spend

* Add test with dummy_tx script_spend

* Add rpc tests for key_spend and script_spend

* Add wrappers for aggregate_nonces and aggregate_partial_signatures, formatting

* Cargo fmt

* Small change for config file

* Some refactor

* (WIP) Change XOnlyPublicKeys to PublicKeys in config files

* Fix errors from pk-xonlypk change

* More refactor

* Remove code duplication

* More refactor

* Commentation + more refactor

* Trying something

* Removed Cargo.lock + add to .gitignore

* Create XOnlyPublicKeys from PublicKeys using an extension trait (#216)

Reduces code duplication and isolates the reponsibility to a dedicated function

* Finalize new architecture for deposit except db operations (#211)

* WIP

* Fix some errors

* WIP

* Merge ozan/new-architecture-improvments into ekrem/new-architecture-improvments (#218)

* WIP Impl. new endpoints

* WIP Impl. new endpoints

* nits

* Implement move txs partial sig

* WIP db nonce

* Use UTXO instead of PsbtOutpoint

* remove redundant db funcs

---------

Co-authored-by: ozankaymak <92448699+ozankaymak@users.noreply.github.com>

* Update DB for New Architecture (#222)

* Change the db file for the new architecture

* Change some UTXOs to OutPoints, add some checks

* WIP Fix inconsistencies

* WIP Impl database wrappers

* Update schema

* Fix some db

* WIP

* nits

* Fix error for a wrapper, add some example db trait impl. and db func.

* nits

* WIP Impl. database functions

---------

Co-authored-by: Ekrem BAL <mail.ekrembal@gmail.com>

* Merge DB changes to ekrem/new-architecture (#227)

* Update config files and impl. some db tests

* WIP Fix checks and tests

* (WIP) Add test for nonce table, fix small errors

* (WIP) Add more tests for nonces table

* (WIP) Add tests for get_pub_nonce

* (WIP) Add test for deposit_utxo

* Change operator new_deposit with new schema

* WIP

* DecodeRow inside the function

* Current tests work

* WIP

* Fix Json issue with UTXODB

* Clean wrapper.rs and formatting

* Add tests for deposit_kickoff_generator_txs table

* Apply review

---------

Co-authored-by: Ekrem BAL <mail.ekrembal@gmail.com>

* Remove code duplication (#228)

* Merge withdrawal into ekrem/new-architecture (#234)

* WIP

* nits

* Add sig check for withdrawal (#229)

* Add sig check for withdrawal

* Use tx.verify

---------

Co-authored-by: Ekrem BAL <mail.ekrembal@gmail.com>

* Impl operator paying withdrawal

* Merge ozan/withdrawal-endpoint-test into ekrem-new-architecture (#230)

* wip

* WIP: created operators and verifiers

* Add steps for the flow

* Add withdrawal endpoint test,organize configs

* Flaky but ok

* Fix flakiness

* Remove unnecessary serialization

---------

Co-authored-by: lemonpartee <lemonpartey@proton.me>
Co-authored-by: Ekrem BAL <mail.ekrembal@gmail.com>

* Merge ozan/kickoff-root-operations into withdrawal (#231)

* WIP Implement endpoints with db operations and tests

* wip

* WIP

* WIP

* Implement changes

* Apply review changes

* Refactor the binaries

* Rename, refactor

---------

Co-authored-by: Ekrem BAL <mail.ekrembal@gmail.com>

---------

Co-authored-by: Ekrem BAL <mail.ekrembal@gmail.com>
Co-authored-by: lemonpartee <lemonpartey@proton.me>

* Implement sig_agg helpers (#237)

Co-authored-by: ozankaymak <ozan.kaymak@boun.edu.tr>

* Deposit test (#232)

* wip

* WIP: created operators and verifiers

* Add steps for the flow

* wip

* Test e2e deposit flow

* Undo unrelated changes

* Fix agg nonces and use same deposit

* Adjust test flow after rebase to withdrawal

* Use create verifiers and operators func

* Works until set_operator_funding_utxo_rpc

* Fix create operators and verifiers

* Fix sig aggregation

* nits

* WIP

* WIP MuSig2VerifyError(BadSignature)

* Debug

* Fix error

* Send move_tx + refactor

---------

Co-authored-by: Ekrem BAL <mail.ekrembal@gmail.com>
Co-authored-by: ozankaymak <ozan.kaymak@boun.edu.tr>

* Apply review + refactor (#243)

* Remove unused dependencies (#238)

* Remove unused utils (#239)

* Add docker compose with database (#245)

* Introduce dockerized PostgresSQL database compatible with integration tests

* refactor compose

* Full withdrawal flow (#253)

* Save operator take and slash or take sigs

* Operator can find kickoff utxo

* Operator can send kickoff tx onchain

* WIP verifiers and operators should use same db missing

* Fix bugs

* WIP Boths sigs verified but operator_take_tx fails

* WIP MuSig2 fix

* Fix withdrawal happy part test

* nits

* Fix musig test

* Use key aggregator trait

---------

Co-authored-by: ozankaymak <ozan.kaymak@boun.edu.tr>

* get rid of basic workflow

* Workflow improvements (#251) (#255)

* workflow: Add all-targets flag to clippy.

* workflow: Add the new build_and_test workflow.

* workflow: Remove all features and move services.

* workflow: Remove old basic workflow.

* workflow: Add cache warm ups for build and test.

* workflow: Add all features flag to build and test.

* workflow: Lower healt-retries to 5.

Co-authored-by: Ceyhun Şen <ceyhuusen@gmail.com>

* workflow: remove all features.

* Merge "clippy" into "ekrem/new-architecture"

* Fix CI config toml

* Change CI config + remove warnings

* Apply clippy suggestions for all

* nits

* Fix duplication

* Fix CI error

* Apply more clippy suggestions (#256)

* cargo: Update mock_rpc.

* workflow: Re-add --all-features.

* workflow: Upgrade 4 cores to 16 cores.

* perf: Insert nonces in a single batch (#248)

* Cargo upgrade

* minimize op_return usage

* fmt

---------

Co-authored-by: ozankaymak <92448699+ozankaymak@users.noreply.github.com>
Co-authored-by: lemonpartee <lemonpartey@proton.me>
Co-authored-by: ozankaymak <ozan.kaymak@boun.edu.tr>
Co-authored-by: Ceyhun Şen <ceyhuusen@gmail.com>
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.

2 participants