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

CSUB-348 Simplify associated signing types #815

Merged
merged 3 commits into from
Jan 3, 2023
Merged

Conversation

CAGS295
Copy link
Contributor

@CAGS295 CAGS295 commented Dec 13, 2022

Description of proposed changes:
Get rid of some associated types that were used as intermediate types between authority key selection and offchain-worker signing.

This may not be final but, the changes are in the right direction.

Practical tips for PR review & merge:

  • All GitHub Actions report PASS
  • Newly added code/functions have unit tests
    • Coverage tools report all newly added lines as covered
    • The positive scenario is exercised
    • Negative scenarios are exercised, e.g. assert on all possible errors
    • Assert on events triggered if applicable
    • Assert on changes made to storage if applicable
  • Modified behavior/functions - try to make sure above test items are covered
  • Integration tests are added if applicable/needed

@github-actions
Copy link

For full LLVM coverage report click here!

@codecov-commenter
Copy link

codecov-commenter commented Dec 13, 2022

Codecov Report

Merging #815 (482410c) into dev (d21ef32) will decrease coverage by 0.01%.
The diff coverage is 96.87%.

@@            Coverage Diff             @@
##              dev     #815      +/-   ##
==========================================
- Coverage   78.06%   78.05%   -0.02%     
==========================================
  Files          66       66              
  Lines       10601    10609       +8     
==========================================
+ Hits         8276     8281       +5     
- Misses       2325     2328       +3     
Impacted Files Coverage Δ
pallets/creditcoin/src/mock.rs 100.00% <ø> (ø)
...allets/offchain-task-scheduler/src/mock/runtime.rs 100.00% <ø> (ø)
pallets/offchain-task-scheduler/src/tasks.rs 100.00% <ø> (ø)
runtime/src/lib.rs 23.78% <ø> (+0.14%) ⬆️
pallets/offchain-task-scheduler/src/lib.rs 95.65% <85.71%> (-1.28%) ⬇️
pallets/creditcoin/src/ocw/tasks/collect_coins.rs 98.95% <100.00%> (ø)
pallets/offchain-task-scheduler/src/ocw.rs 100.00% <100.00%> (ø)
pallets/offchain-task-scheduler/src/ocw/nonce.rs 99.37% <100.00%> (ø)
pallets/offchain-task-scheduler/src/ocw/tests.rs 100.00% <100.00%> (ø)
node/src/main.rs 16.66% <0.00%> (ø)
... and 1 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@CAGS295
Copy link
Contributor Author

CAGS295 commented Dec 19, 2022

blocked by #813

Copy link
Collaborator

@atodorov atodorov left a comment

Choose a reason for hiding this comment

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

Minor changes requested.

.github/dependabot.yml Outdated Show resolved Hide resolved
.github/dependabot.yml Outdated Show resolved Hide resolved
@CAGS295 CAGS295 force-pushed the feature/CSUB-348 branch 2 times, most recently from 39a634f to d3be99b Compare December 20, 2022 23:18
atodorov
atodorov previously approved these changes Dec 21, 2022
Copy link
Collaborator

@atodorov atodorov left a comment

Choose a reason for hiding this comment

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

LGTM

@atodorov atodorov merged commit 80ed87a into dev Jan 3, 2023
@atodorov atodorov deleted the feature/CSUB-348 branch January 3, 2023 11:06
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.

None yet

4 participants