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

[featue] move events module to sbtc crate #1144

Merged
merged 20 commits into from
Dec 18, 2024
Merged

Conversation

MCJOHN974
Copy link
Collaborator

@MCJOHN974 MCJOHN974 commented Dec 16, 2024

Description

Closes #1016

Changes

Testing Information

Checklist:

  • I have performed a self-review of my code
  • My changes generate no new warnings
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@MCJOHN974 MCJOHN974 requested a review from djordon December 16, 2024 13:44
@MCJOHN974 MCJOHN974 marked this pull request as ready for review December 16, 2024 13:45
@MCJOHN974
Copy link
Collaborator Author

Yeah, I also don't like creating 3rd PR for same issue but I guess this commit considers all previous notes and is good enough now

signer/src/storage/mod.rs Outdated Show resolved Hide resolved
signer/src/storage/model.rs Outdated Show resolved Hide resolved
signer/src/storage/model.rs Outdated Show resolved Hide resolved
signer/src/storage/model.rs Outdated Show resolved Hide resolved
signer/src/storage/model.rs Outdated Show resolved Hide resolved
signer/src/storage/model.rs Outdated Show resolved Hide resolved
signer/src/storage/model.rs Outdated Show resolved Hide resolved
signer/src/storage/model.rs Outdated Show resolved Hide resolved
sbtc/src/events.rs Outdated Show resolved Hide resolved
signer/src/api/new_block.rs Outdated Show resolved Hide resolved
@MCJOHN974 MCJOHN974 force-pushed the viktar/moving-event-module-3 branch from 2dee9ed to c991972 Compare December 17, 2024 11:30
@MCJOHN974
Copy link
Collaborator Author

@djordon I believe few commits above fixes all review notes except "We should use local types for key rotation events ". For this note I'm wondering if we want to have any difference between local and sbtc key rotation events. If yes than how this differences should look like.

Copy link
Collaborator

@djordon djordon left a comment

Choose a reason for hiding this comment

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

I realized that we'll also want to move the signer/src/stacks/webhooks.rs file over to the sbtc crate as well. You'll should feature gate the webhooks module though and if you like I can help with that.

signer/src/storage/model.rs Outdated Show resolved Hide resolved
signer/src/storage/model.rs Outdated Show resolved Hide resolved
signer/src/storage/model.rs Outdated Show resolved Hide resolved
signer/src/storage/model.rs Outdated Show resolved Hide resolved
signer/src/storage/model.rs Outdated Show resolved Hide resolved
signer/src/testing/dummy.rs Outdated Show resolved Hide resolved
signer/src/testing/dummy.rs Outdated Show resolved Hide resolved
signer/src/testing/dummy.rs Outdated Show resolved Hide resolved
signer/tests/integration/postgres.rs Outdated Show resolved Hide resolved
signer/src/request_decider.rs Outdated Show resolved Hide resolved
@MCJOHN974
Copy link
Collaborator Author

I realized that we'll also want to move the signer/src/stacks/webhooks.rs file over to the sbtc crate as well. You'll should feature gate the webhooks module though and if you like I can help with that.

Maybe this pr is in progress for too long and refactoring webhooks and events are different enough tasks to make them in separate pr

@MCJOHN974
Copy link
Collaborator Author

MCJOHN974 commented Dec 18, 2024

@djordon I think commits above fix new issues and we can go back to question "how local KeyRotationEvent should look like?", if we still want to have another local type

upd: I think local KeyRotationEvent I suggested in next commit quite fits to style we used to create local types for other events

@MCJOHN974 MCJOHN974 requested a review from djordon December 18, 2024 18:37
Copy link
Collaborator

@djordon djordon left a comment

Choose a reason for hiding this comment

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

Looks good ✅

@MCJOHN974 MCJOHN974 merged commit 6b7e313 into main Dec 18, 2024
4 checks passed
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.

[Feature]: Move sBTC event module to the sbtc crate
2 participants