Skip to content

Randomness monitor service#584

Merged
GabrielMartinezRodriguez merged 12 commits intomasterfrom
gabriel/randomness-monitor
Apr 29, 2025
Merged

Randomness monitor service#584
GabrielMartinezRodriguez merged 12 commits intomasterfrom
gabriel/randomness-monitor

Conversation

@GabrielMartinezRodriguez
Copy link
Contributor

@GabrielMartinezRodriguez GabrielMartinezRodriguez commented Apr 3, 2025 •
edited
Loading

Description

This PR adds a new service that monitors every block of our blockchain to check whether the random() function is available. It then saves the result in a SQLite database, which is later queried by Grafana to display this dashboard: https://grafana.happy.tech/goto/ZamzFWAHR?orgId=1

Toggle Checklist

Checklist

Basics

  • B1. I have applied the proper label & proper branch name (e.g. norswap/build-system-caching).
  • B2. This PR is not so big that it should be split & addresses only one concern.
  • B3. The PR targets the lowest branch it can (ideally master).

Reminder: PR review guidelines

Correctness

  • C1. Builds and passes tests.
  • C2. The code is properly parameterized & compatible with different environments (e.g. local,
    testnet, mainnet, standalone wallet, ...).
  • C3. I have manually tested my changes & connected features.

< INDICATE BROWSER, DEMO APP & OTHER ENV DETAILS USED FOR TESTING HERE >

< INDICATE TESTED SCENARIOS (USER INTERFACE INTERACTION, CODE FLOWS) HERE >

  • C4. I have performed a thorough self-review of my code after submitting the PR,
    and have updated the code & comments accordingly.

Architecture & Documentation

  • D1. I made it easy to reason locally about the code, by (1) using proper abstraction boundaries,
    (2) commenting these boundaries correctly, (3) adding inline comments for context when needed.
  • D2. All public-facing APIs & meaningful (non-local) internal APIs are properly documented in code
    comments.
  • D3. If appropriate, the general architecture of the code is documented in a code comment or
    in a Markdown document.
  • D4. An appropriate Changeset has been generated (and committed) for changes that touch npm published packages (currently packages/core and packages/react), see here for more info.

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Apr 3, 2025 •
edited
Loading

Deploying happychain with  Cloudflare Pages  Cloudflare Pages

Latest commit: cb1dae3
Status:⚡️  Build in progress...

View logs

This was referenced Apr 3, 2025
Merged
Merged
Merged
This was referenced Apr 3, 2025
Merged
Merged
Merged
Merged
@GabrielMartinezRodriguez GabrielMartinezRodriguez added the reviewing-1 Ready for, or undergoing first-line review label Apr 3, 2025
This was referenced Apr 3, 2025
Merged
Merged
@GabrielMartinezRodriguez Graphite App
Copy link
Contributor Author

GabrielMartinezRodriguez commented Apr 3, 2025 •
edited
Loading

This stack of pull requests is managed by Graphite. Learn more about stacking.

@GabrielMartinezRodriguez GabrielMartinezRodriguez mentioned this pull request Apr 3, 2025
Merged
11 tasks
@GabrielMartinezRodriguez GabrielMartinezRodriguez added draft Not ready for review and removed reviewing-1 Ready for, or undergoing first-line review labels Apr 3, 2025
aodhgan
aodhgan reviewed Apr 3, 2025
View reviewed changes
apps/randomness-monitor/src/index.ts Outdated
blockNumber: blockNumber,
})
console.log(random)
result = new Monitoring(blockNumber, block.timestamp, MonitoringResult.Success)
Copy link
Contributor

@aodhgan aodhgan Apr 3, 2025

Choose a reason for hiding this comment

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

i think we should also save the randomness value itself? since we are fetching it here, it may be useful in future analysis.

Copy link
Contributor Author

@GabrielMartinezRodriguez GabrielMartinezRodriguez Apr 7, 2025

Choose a reason for hiding this comment

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

Added

aodhgan
aodhgan reviewed Apr 3, 2025
View reviewed changes
apps/randomness-monitor/src/index.ts Outdated
result = new Monitoring(blockNumber, block.timestamp, MonitoringResult.Failure)
}

await this.monitoringRepository.saveMonitoring(result)
Copy link
Contributor

@aodhgan aodhgan Apr 3, 2025

Choose a reason for hiding this comment

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

this can fail, right?

@GabrielMartinezRodriguez GabrielMartinezRodriguez added reviewing-1 Ready for, or undergoing first-line review and removed draft Not ready for review labels Apr 4, 2025
@GabrielMartinezRodriguez GabrielMartinezRodriguez marked this pull request as ready for review April 4, 2025 14:18
@GabrielMartinezRodriguez GabrielMartinezRodriguez changed the title feat(randomness-monitor): randomness monitor service Randomness monitor service Apr 4, 2025
@GabrielMartinezRodriguez GabrielMartinezRodriguez force-pushed the gabriel/randomness-monitor branch 2 times, most recently from 43caf65 to 1194438 Compare April 7, 2025 11:57
Base automatically changed from gabriel/txm-traces to master April 13, 2025 21:42
not-reed
not-reed reviewed Apr 14, 2025
View reviewed changes
This was referenced Apr 21, 2025
Merged
Merged
Merged
@GabrielMartinezRodriguez GabrielMartinezRodriguez mentioned this pull request Apr 28, 2025
Merged
11 tasks
norswap
norswap approved these changes Apr 29, 2025
View reviewed changes
apps/randomness-monitor/src/utils/getUrlProtocol.ts
Copy link
Collaborator

@norswap norswap Apr 29, 2025

Choose a reason for hiding this comment

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

I feel like I've seen this somewhere else, should we move to common?

Copy link
Contributor Author

@GabrielMartinezRodriguez GabrielMartinezRodriguez Apr 29, 2025

Choose a reason for hiding this comment

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

done

apps/randomness-monitor/src/index.ts Outdated
this.latestMonitoringBlockNumber = this.latestBlockchainBlockNumber
}

if (this.latestMonitoringBlockNumber && this.latestBlockchainBlockNumber) {
Copy link
Collaborator

@norswap norswap Apr 29, 2025

Choose a reason for hiding this comment

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

That condition is guaranteed by the two if above.

Copy link
Contributor Author

@GabrielMartinezRodriguez GabrielMartinezRodriguez Apr 29, 2025

Choose a reason for hiding this comment

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

right

apps/randomness-monitor/src/index.ts Outdated
* Monitor a specific block for randomness
*/
private async monitorBlock(blockNumber: bigint): Promise<Result<void, Error>> {
console.log("Monitoring block", blockNumber)
Copy link
Collaborator

@norswap norswap Apr 29, 2025

Choose a reason for hiding this comment

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

use logger

apps/randomness-monitor/src/index.ts
Copy link
Collaborator

@norswap norswap Apr 29, 2025

Choose a reason for hiding this comment

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

I agree this is a monitoring service, but probably could use a better name for "a monitoring", what about "a check"?

Copy link
Contributor Author

@GabrielMartinezRodriguez GabrielMartinezRodriguez Apr 29, 2025

Choose a reason for hiding this comment

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

done

apps/randomness-monitor/src/index.ts
Copy link
Collaborator

@norswap norswap Apr 29, 2025

Choose a reason for hiding this comment

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

This could probably use a README file to explain the purpose, doesn't need to be much, maybe a slight expansion over the PR description.

Another question: how is the Grafana integration working, is there code for that somewhere?

Copy link
Contributor Author

@GabrielMartinezRodriguez GabrielMartinezRodriguez Apr 29, 2025

Choose a reason for hiding this comment

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

README added. Regarding your question about Grafana: the randomness monitoring service saves the results in a SQLite database, and Grafana queries that database directly to display the dashboard and configure alerts

@GabrielMartinezRodriguez GabrielMartinezRodriguez merged commit 00738d1 into master Apr 29, 2025
1 of 2 checks passed
@GabrielMartinezRodriguez GabrielMartinezRodriguez deleted the gabriel/randomness-monitor branch April 29, 2025 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aodhgan aodhgan aodhgan left review comments

@norswap norswap norswap approved these changes

@not-reed not-reed not-reed approved these changes

Assignees

@GabrielMartinezRodriguez GabrielMartinezRodriguez

Labels

reviewing-1 Ready for, or undergoing first-line review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@GabrielMartinezRodriguez @norswap @aodhgan @not-reed