search

tangle-network / dkg-substrate

Multy-party threshold ECDSA (GG20) Substrate node
https://tangle.webb.tools/
GNU General Public License v3.0
60 stars 15 forks source link

Add WT-FROST to the worker gadget #711

Closed tbraun96 closed 1 year ago

tbraun96 commented 1 year ago

Closes #710

codecov-commenter commented 1 year ago

Codecov Report

Attention: 354 lines in your changes are missing coverage. Please review.

Comparison is base (da48f39) 17.91% compared to head (a873b3f) 17.11%.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #711 +/- ## ========================================== - Coverage 17.91% 17.11% -0.79% ========================================== Files 77 80 +3 Lines 5696 5956 +260 ========================================== - Hits 1020 1019 -1 - Misses 4676 4937 +261 ``` | [Files](https://app.codecov.io/gh/webb-tools/dkg-substrate/pull/711?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=webb-tools) | Coverage Δ | | |---|---|---| | [...gadget/src/async\_protocols/ecdsa/keygen/handler.rs](https://app.codecov.io/gh/webb-tools/dkg-substrate/pull/711?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=webb-tools#diff-ZGtnLWdhZGdldC9zcmMvYXN5bmNfcHJvdG9jb2xzL2VjZHNhL2tleWdlbi9oYW5kbGVyLnJz) | `0.00% <ø> (ø)` | | | [...et/src/async\_protocols/ecdsa/sign/state\_machine.rs](https://app.codecov.io/gh/webb-tools/dkg-substrate/pull/711?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=webb-tools#diff-ZGtnLWdhZGdldC9zcmMvYXN5bmNfcHJvdG9jb2xzL2VjZHNhL3NpZ24vc3RhdGVfbWFjaGluZS5ycw==) | `0.00% <ø> (ø)` | | | [dkg-gadget/src/async\_protocols/mod.rs](https://app.codecov.io/gh/webb-tools/dkg-substrate/pull/711?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=webb-tools#diff-ZGtnLWdhZGdldC9zcmMvYXN5bmNfcHJvdG9jb2xzL21vZC5ycw==) | `0.00% <ø> (ø)` | | | [dkg-gadget/src/storage/proposals.rs](https://app.codecov.io/gh/webb-tools/dkg-substrate/pull/711?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=webb-tools#diff-ZGtnLWdhZGdldC9zcmMvc3RvcmFnZS9wcm9wb3NhbHMucnM=) | `0.00% <ø> (ø)` | | | [dkg-gadget/src/db/offchain\_storage.rs](https://app.codecov.io/gh/webb-tools/dkg-substrate/pull/711?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=webb-tools#diff-ZGtnLWdhZGdldC9zcmMvZGIvb2ZmY2hhaW5fc3RvcmFnZS5ycw==) | `0.00% <0.00%> (ø)` | | | [dkg-gadget/src/worker.rs](https://app.codecov.io/gh/webb-tools/dkg-substrate/pull/711?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=webb-tools#diff-ZGtnLWdhZGdldC9zcmMvd29ya2VyLnJz) | `0.00% <0.00%> (ø)` | | | [dkg-gadget/src/dkg\_modules/mp\_ecdsa.rs](https://app.codecov.io/gh/webb-tools/dkg-substrate/pull/711?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=webb-tools#diff-ZGtnLWdhZGdldC9zcmMvZGtnX21vZHVsZXMvbXBfZWNkc2EucnM=) | `0.00% <0.00%> (ø)` | | | [.../src/async\_protocols/ecdsa/keygen/state\_machine.rs](https://app.codecov.io/gh/webb-tools/dkg-substrate/pull/711?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=webb-tools#diff-ZGtnLWdhZGdldC9zcmMvYXN5bmNfcHJvdG9jb2xzL2VjZHNhL2tleWdlbi9zdGF0ZV9tYWNoaW5lLnJz) | `0.00% <0.00%> (ø)` | | | [dkg-gadget/src/dkg\_modules/mod.rs](https://app.codecov.io/gh/webb-tools/dkg-substrate/pull/711?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=webb-tools#diff-ZGtnLWdhZGdldC9zcmMvZGtnX21vZHVsZXMvbW9kLnJz) | `0.00% <0.00%> (ø)` | | | [...g-gadget/src/async\_protocols/ecdsa/sign/handler.rs](https://app.codecov.io/gh/webb-tools/dkg-substrate/pull/711?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=webb-tools#diff-ZGtnLWdhZGdldC9zcmMvYXN5bmNfcHJvdG9jb2xzL2VjZHNhL3NpZ24vaGFuZGxlci5ycw==) | `0.00% <0.00%> (ø)` | | | ... and [7 more](https://app.codecov.io/gh/webb-tools/dkg-substrate/pull/711?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=webb-tools) | | ... and [3 files with indirect coverage changes](https://app.codecov.io/gh/webb-tools/dkg-substrate/pull/711/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=webb-tools)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

drewstone commented 1 year ago

Maybe you're missing some feature flags @tbraun96 ? What ultimately changed with how our integration tests run?

tbraun96 commented 1 year ago

What ultimately changed with how our integration tests run?

So far, nothing changed in how our integration tests are run. Relevantly, what has changed is that our FROST dependency needs to link to a C library, and it appears it lacks testing for pipeline builds (cargo test does not count, as the linking process is different. What is failing for us here is when we build binaries via cargo run or cargo build). I am in contact with @xoloki about this situation and am exploring ways to fix this issue. We may need a custom build.rs script for our frost dependency.

xoloki commented 1 year ago

I'm really not sure what's going on here. In stacks-sbtc we link wsts with the p256k1 dependency, and CI works fine on x86_64 and arm64 for both linux and macos. All we needed to do was add libclang-dev to our build pipeline.

I'll pull this repo and try to see what's going on. p256k1 has its own build.rs, which should be adequate on all platforms.

xoloki commented 1 year ago

I'll pull this repo and try to see what's going on. p256k1 has its own build.rs, which should be adequate on all platforms.

Just pulled and built thomas/frost-dkg, no problems. I just had to install protobuf-compiler and libgmp-dev.

@tbraun96 does this fail for you locally, or just in GH CI?

drewstone commented 1 year ago

Just in CI @xoloki

xoloki commented 1 year ago

@drewstone @tbraun96 there seems to be a problem in how CI is setup and run.

I would expect that the Harness CI target comes from .github/workflows/harness_stress_tests.yml. But in that file, the Install Protobuf and LLVM step should run bitcoin.sh then sudo apt-get install protobuf-compiler llvm libsecp256k1-dev libsecp256k1-0 libclang-dev.

However, that's not what happens. The bitcoin.sh script does not run, and apt-get is run twice, once for protobuf-compiler and once for llvm. That's why p256k1 is failing to link, because libclang-dev is not being installed.

xoloki commented 1 year ago

However, for the Integration tests / Local Chain Tests target, we are actually installing libclang-dev, but the linker problem persists.

There are a couple of things about your CI setup which I note. First, you are using sccache for both failing tests. Second, you are using cargo-make for the integration test. Maybe remove these and try a basic cargo build step in each?

xoloki commented 1 year ago

I'd also remove the libsecp256k1 install calls, unless you're also using those for something else.

Basically, I'd just remove everything in the failing CI .yml files and build them from scratch, installing only what you need and seeing if cargo build works. Once that works then you can add stuff back in one piece at a time.

tbraun96 commented 1 year ago

The issue, as you hinted towards @xoloki , was sccache. Thank you so much for the help.

xoloki commented 1 year ago

The issue, as you hinted towards @xoloki , was sccache. Thank you so much for the help.

Glad I could help 😎