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

feat: Update sBTC best practices #1709

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

aldur
Copy link
Contributor

@aldur aldur commented Feb 26, 2025

Description

Describe the changes that were made in this pull request. When possible start with the motivations behind the change.

@aldur aldur requested a review from a team as a code owner February 26, 2025 10:35
@aldur aldur marked this pull request as draft February 26, 2025 10:35
Copy link
Member

@cylewitruk cylewitruk left a comment

Choose a reason for hiding this comment

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

Lgtm in general for this revision, just had a few suggestions.

Comment on lines 61 to 62
- Stay up-to-date with new releases, patches, and security advisories (e.g.,
GitHub, mailing lists, Discord).
Copy link
Member

@cylewitruk cylewitruk Feb 26, 2025

Choose a reason for hiding this comment

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

Suggestion (just tossed this together, but I guess you get the idea I'm going for):

Suggested change
- Stay up-to-date with new releases, patches, and security advisories (e.g.,
GitHub, mailing lists, Discord).
- Stay up-to-date with new releases, patches, and security advisories for all used operating systems, software and packages:
- https://www.cve.org/ is a great resource for popular software packages.
- Subscribe to receive security notifications from your vendors.
- Join relevant messaging channels as applicable (i.e. on Discord, Slack, etc.).

## Monitor new software releases

- Stay up-to-date with new releases, patches, and security advisories (e.g.,
GitHub, mailing lists, Discord).
- Exercise vulnerability management for all packages.
- Apply updates as quickly as possible, especially those addressing a security
vulnerability.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
vulnerability.
vulnerability.
- Use inventory and patch management software, if available.

Comment on lines +15 to +16
- Periodically verify the integrity of backups (e.g. by importing them into a
fresh PostgreSQL instance).

Choose a reason for hiding this comment

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

Should we expand a bit on the verify? I think we don't want people restoring backup on prod running instances, but not even running parallel prod instances with the same config and old db snapshot (connected to the same signer network). OTOH, it may not be clear what to check after an import (eg, maybe it's a schema only backup, so it doesn't fail but is empty)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Heard, should we tell them to spin up an empty PSQL DB, restore the DB and count the aggregate shares?

Choose a reason for hiding this comment

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

Yeah given that that one is the table we are mostly interested in, we could ask them to query for the latest share and check its status and aggregate key. For the aggregate key we would want that to match the current one in the smart contract, eg fetching it via:

curl 'https://api.hiro.so/v2/contracts/call-read/SM3VDXK3WZZSA84XXFKAFAF15NNZX32CTSG82JFQ4/sbtc-registry/get-current-aggregate-pubkey' -H 'content-type: application/json' --data-raw '{"sender":"SM3VDXK3WZZSA84XXFKAFAF15NNZX32CTSG82JFQ4","arguments":[]}'

unless there was a recent rotate key, then it would not match.

A possible issue with the row count is that now it may not be informative, as if we fail DKG verification we would retry, so there may be more rows than expected. But I guess we could just ask them to ensure it's non decreasing maybe?

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.

3 participants