Skip to content

feat(randomness): drand prune#693

Merged
norswap merged 1 commit intomasterfrom
gabriel/drand-prune
May 8, 2025
Merged

feat(randomness): drand prune#693
norswap merged 1 commit intomasterfrom
gabriel/drand-prune

Conversation

@GabrielMartinezRodriguez
Copy link
Contributor

@GabrielMartinezRodriguez GabrielMartinezRodriguez commented Apr 28, 2025

Description

Prune the drand repository every 20 seconds to avoid having thousands of drands in memory, which causes delays when trying to submit drands

  • Include all relevant context (but no need to repeat the issue's content).
  • Draw attention to new, noteworthy & unintuitive elements.
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 are documented in the docs.
    Public APIS and 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) with make changeset for
    breaking and meaningful changes in packages (not required for cleanups & refactors).

Copy link
Contributor Author

GabrielMartinezRodriguez commented Apr 28, 2025

This was referenced Apr 28, 2025
@GabrielMartinezRodriguez GabrielMartinezRodriguez marked this pull request as ready for review April 28, 2025 11:14
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Apr 28, 2025

Deploying happychain with  Cloudflare Pages  Cloudflare Pages

Latest commit: 15884ab
Status: ✅  Deploy successful!
Preview URL: https://a61aaecd.happychain.pages.dev
Branch Preview URL: https://gabriel-drand-prune.happychain.pages.dev

View logs

@GabrielMartinezRodriguez GabrielMartinezRodriguez added the reviewing-1 Ready for, or undergoing first-line review label Apr 28, 2025
Base automatically changed from gabriel/txm-value-parameter to master April 28, 2025 12:58
to: transaction.address,
data: transaction.calldata,
value: 0n,
value: transaction.value,
Copy link
Contributor

Choose a reason for hiding this comment

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

this change is already in master

Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems like a git bug — I've seen happen quite a bit recently, I usually fix it by restacking and resubmitting.

``` No newline at end of file
<<<<<<< HEAD
```
=======
Copy link
Contributor

Choose a reason for hiding this comment

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

uh oh dodgy rebase

@aodhgan
Copy link
Contributor

aodhgan commented May 8, 2025

think this had some funky rebase/merge conflict resolution, since a bunch of allowing transaction values to be non-zero is included. either worth trying to resolve or re-opening with just the specific intentional changes (drand pruning)
also changes contracts/package.json which it shouldnt etc.

@aodhgan aodhgan added question Something has to be cleared up after review and removed reviewing-1 Ready for, or undergoing first-line review labels May 8, 2025
@norswap norswap force-pushed the gabriel/drand-prune branch from 4e9cd77 to 15884ab Compare May 8, 2025 21:02
@norswap
Copy link
Collaborator

norswap commented May 8, 2025

think this had some funky rebase/merge conflict resolution, since a bunch of allowing transaction values to be non-zero is included. either worth trying to resolve or re-opening with just the specific intentional changes (drand pruning) also changes contracts/package.json which it shouldnt etc.

As mentionned on other comment, Github sucking — restacked and resubmitted, removes all that cruft.

@norswap norswap merged commit e89314f into master May 8, 2025
3 checks passed
@norswap norswap deleted the gabriel/drand-prune branch May 8, 2025 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

question Something has to be cleared up after review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants