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

[fix] Address sbp review #491

Merged
merged 28 commits into from
Mar 10, 2023
Merged

[fix] Address sbp review #491

merged 28 commits into from
Mar 10, 2023

Conversation

1xstj
Copy link
Contributor

@1xstj 1xstj commented Feb 9, 2023

Summary of changes
Changes introduced in this pull request:

  • Fix unbounded storage used in the runtime and dkg-gadget
  • Other misc fixes mentioned in sbp review

Depends on tangle-network/webb-rs#117 / tangle-network/webb-rs#121

Reference issue to close (if applicable)
Closes #471

@1xstj 1xstj force-pushed the patch/sbp-fixes branch from 64ccf76 to e7c3fa0 Compare March 7, 2023 20:20
@drewstone
Copy link
Contributor

Status of this @1xstj, can we get this wrapped up this week? Seems the errors are straightforward at first glance.

@1xstj 1xstj force-pushed the patch/sbp-fixes branch from de062f2 to 339375f Compare March 9, 2023 05:05
@codecov-commenter
Copy link

codecov-commenter commented Mar 10, 2023

Codecov Report

Patch coverage: 45.11% and no project coverage change.

Comparison is base (e37ef09) 21.34% compared to head (a1d926a) 21.33%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #491      +/-   ##
==========================================
- Coverage   21.34%   21.33%   -0.00%     
==========================================
  Files          67       67              
  Lines        4317     4426     +109     
==========================================
+ Hits          921      944      +23     
- Misses       3396     3482      +86     
Impacted Files Coverage Δ
...gadget/src/async_protocols/blockchain_interface.rs 0.00% <0.00%> (ø)
dkg-gadget/src/async_protocols/incoming.rs 0.00% <ø> (ø)
dkg-gadget/src/async_protocols/keygen/handler.rs 0.00% <0.00%> (ø)
...gadget/src/async_protocols/keygen/state_machine.rs 0.00% <ø> (ø)
dkg-gadget/src/async_protocols/mod.rs 0.00% <0.00%> (ø)
dkg-gadget/src/async_protocols/sign/handler.rs 0.00% <ø> (ø)
...g-gadget/src/async_protocols/sign/state_machine.rs 0.00% <ø> (ø)
dkg-gadget/src/async_protocols/state_machine.rs 0.00% <ø> (ø)
...adget/src/async_protocols/state_machine_wrapper.rs 0.00% <ø> (ø)
...-gadget/src/gossip_messages/misbehaviour_report.rs 0.00% <0.00%> (ø)
... and 16 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@1xstj 1xstj marked this pull request as ready for review March 10, 2023 12:02
Copy link
Contributor

@shekohex shekohex left a comment

Choose a reason for hiding this comment

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

LGTM! Except all these try_into().unwrap() everywhere, we shall handle these as errors and add a new Error variant to the DKGError for example.

dkg-runtime-primitives/src/lib.rs Outdated Show resolved Hide resolved
pallets/dkg-metadata/src/mock.rs Outdated Show resolved Hide resolved
@1xstj
Copy link
Contributor Author

1xstj commented Mar 10, 2023

LGTM! Except all these try_into().unwrap() everywhere, we shall handle these as errors and add a new Error variant to the DKGError for example.

Opened a task so we dont forget #521

Copy link
Contributor

@shekohex shekohex left a comment

Choose a reason for hiding this comment

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

LG!

@1xstj 1xstj merged commit 66c42e3 into master Mar 10, 2023
@1xstj 1xstj deleted the patch/sbp-fixes branch March 10, 2023 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Completed ✅
Development

Successfully merging this pull request may close these issues.

4 participants