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.

Sign up for GitHub

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

Jump to bottom

Add restriction per for lightningd runes #6617

Merged
merged 6 commits into from
Sep 21, 2023
Merged

Add restriction per for lightningd runes #6617

merged 6 commits into from
Sep 21, 2023

Conversation

ShahanaFarooqui
Copy link
Collaborator

No description provided.

@rustyrussell
Copy link
Contributor 

wallet/wallet.c:5689:	else 
Extraneous whitespace found
make: *** [Makefile:522: check-whitespace/wallet/wallet.c] Error 1

rustyrussell
rustyrussell reviewed Aug 26, 2023
View reviewed changes
Copy link
Contributor

@rustyrussell rustyrussell left a comment

Choose a reason for hiding this comment

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

Minor comments, and importantly, still needs test for per...

wallet/db.c Outdated Show resolved Hide resolved
wallet/wallet.c Outdated Show resolved Hide resolved
lightningd/runes.c Outdated Show resolved Hide resolved
common/json_stream.c Show resolved Hide resolved
@rustyrussell rustyrussell mentioned this pull request Aug 26, 2023
Merged
@ShahanaFarooqui ShahanaFarooqui marked this pull request as draft August 28, 2023 01:38
@ShahanaFarooqui ShahanaFarooqui force-pushed the runes-per-restriction branch 3 times, most recently from b22e50b to 1ab4d91 Compare September 5, 2023 01:09
@rustyrussell rustyrussell added this to the v23.11 milestone Sep 14, 2023
@ShahanaFarooqui ShahanaFarooqui marked this pull request as ready for review September 15, 2023 17:48
@rustyrussell
Copy link
Contributor

Ack f7143a0

@rustyrussell
Copy link
Contributor

Ah, you can't call your helper function test_... since pytest tries to run that. Simple rename pushed.

@ShahanaFarooqui @rustyrussell
lightning/runes: added last_used in showrunes
99fea9d 
Changelog-Added: JSON-RPC: `showrunes` new field `last_used`
@ShahanaFarooqui @rustyrussell
common: make json_add_timeabs full precision & renamed json_add_time
b7033f8 
json_add_timeabs only printed in milliseconds and json_add_time outputs a string which is weird

Changelog-Changed: JSON-RPC time fields now have full nanosecond precision (i.e. 9 decimals not 3): `listfowards` `received_time` `resolved_time` `listpays`/`listsendpays` `created_at`.
@ShahanaFarooqui @rustyrussell
lightningd/runes: Added last_time_check for validating rune for per…
b143737 
… restriction

Changelog-Added: runes: `per=Nsec/min/hour/msec/usec/nsec` for general ratelimiting
@ShahanaFarooqui @rustyrussell
tests/runes: per restriction test cases
968d5d9
@rustyrussell
Copy link
Contributor

Ah, and valgrind caught that we missed initializing a variable (if there's no rune in db, we left last_used uninitialized for json_add_rune).

Fixed. This time for sure!

@rustyrussell rustyrussell merged commit f60882f into ElementsProject:master Sep 21, 2023
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rustyrussell rustyrussell rustyrussell left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v23.11
Development

Successfully merging this pull request may close these issues.
2 participants
@ShahanaFarooqui @rustyrussell