Skip to content

dx: Add additional hooks to pre-commit#7

Closed
s373nZ wants to merge 7 commits intocdecker:202449-pre-commitfrom
s373nZ:7765-pre-commit-hooks
Closed

dx: Add additional hooks to pre-commit#7
s373nZ wants to merge 7 commits intocdecker:202449-pre-commitfrom
s373nZ:7765-pre-commit-hooks

Conversation

@s373nZ
Copy link

@s373nZ s373nZ commented Nov 29, 2024 •
edited
Loading

Adds additional hooks to pre-commit.

  • The official shellcheck hook.
  • Re-implements make check-amount-access as a local pygrep hook.
  • Implements a clang-format hook to fail on suggestion warnings with additional configuration to sort includes.
  • Re-implements make check-discouraged-functions as a local pygrep hook.

Adding this PR (along with future hooks to come) against your PR for now, until ElementsProject#7884 is merged.

@s373nZ
Copy link
Author

s373nZ commented Nov 30, 2024

Added an implementation of the make check-amount-access check. Demonstrates a locally defined hook using pygrep and therefore the regex has been converted from piped grep commands into Python format.

@s373nZ
Copy link
Author

s373nZ commented Dec 1, 2024

Added an initial version of a clang-format check. Default behavior is to do in-place editing of the files, but that is overridden and suggestion warnings are reported as errors to force a failure. Output in this fashion isn't exactly easy to see the full context of the suggestion, but one can run the full checks and inspect a diff for more of an overview.

Noticed an existing config in .clang-format and added additional instruction to sort #includes in hopes of eventually replacing make check-src-include-order and make check-hdr-include-order.

Also probably needs some more attention around which files to exclude, in accordance with the SRC_TO_CHECK and ALL_NONGEEN_HEADERS definitions in the Makefile.

@s373nZ
Copy link
Author

s373nZ commented Dec 1, 2024

Added an implementation of the make check-discouraged-functions check, also as a Python regex. There might be another static analysis tool that could also do this (and more?).

@s373nZ s373nZ force-pushed the 7765-pre-commit-hooks branch from b531f46 to 41003ee Compare December 16, 2024 18:14
@s373nZ
Copy link
Author

s373nZ commented Dec 16, 2024

  • Cleaned up regex syntax for earlier hooks.
  • Added codespell to check spelling.

@s373nZ
Copy link
Author

s373nZ commented Dec 17, 2024

  • Added check-jsonschema hooks to validate doc/ JSON schemas and native pre-commit hook for pretty formatting.

@s373nZ
chore: Add amount access check to pre-commit.
4f2e630 
Reimplements `make check-amount-access` for Python regex.
@s373nZ
chore: Add clang-format to pre-commit.
d0eef43 
Also intends to replace `make check-src-include-order` and
`make check-hdr-include-order` by adding configuration to check
for include ordering.
@s373nZ
chore: Add discouraged function check to pre-commit.
a455383 
Reimplements `make check-discouraged-functions` for Python regex.
@s373nZ
chore: Add codespell to pre-commit.
4edc6f5 
Includes default config file and an initial word list to ignore.
@s373nZ
chore: Add doc JSON schema checks and formatting to pre-commit.
6ff1ff2
@s373nZ
chore: Add conventional commit linting to pre-commit.
3265aea
@s373nZ s373nZ force-pushed the 7765-pre-commit-hooks branch from e5560cd to 3265aea Compare December 18, 2024 19:35
@s373nZ
Copy link
Author

s373nZ commented Dec 18, 2024

  • Added conventional commit linting for commit messages. Looks like the also gets processed by codespell.

Grabbed the list of "scopes" from top level directory organization, along with a sane perusal of the commit messages from the last few months. Needs some auditing.

This hook utilization requires running pre-commit install --hook-type commit-msg. Would be cool to brainstorm a way to bootstrap the contributor environment with one command, like a Makefile target or script, if possible.

@s373nZ s373nZ mentioned this pull request Mar 11, 2025
Merged
1 task
@s373nZ
Copy link
Author

s373nZ commented Apr 1, 2025

Closing in favor of ElementsProject#8193 against CLN repo.

@s373nZ s373nZ closed this Apr 1, 2025
cdecker pushed a commit that referenced this pull request Oct 27, 2025
@rustyrussell
plugins: fix %*.s typo.
8f6d3d8 
And add a check for new uses creeping in, since it got cut & paste
everywhere.

This means "this is a valid string, but truncate it to this many characters"
vs "%.*s" which means "only read this many characters of string":

```
['lightningd-3 2025-10-23T02:31:40.890Z **BROKEN** plugin-funder: Plugin marked as important, shutting down lightningd!']
--------------------------- Captured stderr teardown ---------------------------
    #0 0x557da58ad1dc in printf_common(void*, char const*, __va_list_tag*) asan_interceptors.cpp.o
    #1 0x557da5aff814 in json_out_addv /home/runner/work/lightning/lightning/ccan/ccan/json_out/json_out.c:239:11
    #2 0x557da59740ce in plugin_logv /home/runner/work/lightning/lightning/plugins/libplugin.c:1777:2
    #3 0x557da5969b6f in plugin_log /home/runner/work/lightning/lightning/plugins/libplugin.c:1934:2
    #4 0x557da595c4f6 in datastore_del_success /home/runner/work/lightning/lightning/plugins/funder.c:161:2
    #5 0x557da598b837 in handle_rpc_reply /home/runner/work/lightning/lightning/plugins/libplugin.c:1072:10
    #6 0x557da598a4b0 in rpc_conn_read_response /home/runner/work/lightning/lightning/plugins/libplugin.c:1361:3
    #7 0x557da5adbea5 in next_plan /home/runner/work/lightning/lightning/ccan/ccan/io/io.c:60:9
    ElementsProject#8 0x557da5ae06ff in do_plan /home/runner/work/lightning/lightning/ccan/ccan/io/io.c:422:8
    ElementsProject#9 0x557da5adfb58 in io_ready /home/runner/work/lightning/lightning/ccan/ccan/io/io.c:439:10
    ElementsProject#10 0x557da5aec2ce in io_loop /home/runner/work/lightning/lightning/ccan/ccan/io/poll.c:455:5
    ElementsProject#11 0x557da59757ac in plugin_main /home/runner/work/lightning/lightning/plugins/libplugin.c:2409:3
    ElementsProject#12 0x557da594fe23 in main /home/runner/work/lightning/lightning/plugins/funder.c:1723:2
    ElementsProject#13 0x7f6572229d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    ElementsProject#14 0x7f6572229e3f in __libc_start_main csu/../csu/libc-start.c:392:3
    ElementsProject#15 0x557da588b584 in _start (/home/runner/work/lightning/lightning/plugins/funder+0x10d584) (BuildId: 71ba63ab577fc6fa60573d3e8555f6db7d5c584d)

0x624000009d28 is located 0 bytes to the right of 7208-byte region [0x624000008100,0x624000009d28)
allocated by thread T0 here:
    #0 0x557da590e7f6 in __interceptor_realloc (/home/runner/work/lightning/lightning/plugins/funder+0x1907f6) (BuildId: 71ba63ab577fc6fa60573d3e8555f6db7d5c584d)
    #1 0x557da5b2149b in tal_resize_ /home/runner/work/lightning/lightning/ccan/ccan/tal/tal.c:755:13
    #2 0x557da59f2032 in membuf_tal_resize /home/runner/work/lightning/lightning/common/utils.c:203:2
    #3 0x557da5b03934 in membuf_prepare_space_ /home/runner/work/lightning/lightning/ccan/ccan/membuf/membuf.c:45:12
    #4 0x557da59d4289 in jsonrpc_io_read_ /home/runner/work/lightning/lightning/common/jsonrpc_io.c:127:2
    #5 0x557da598a635 in rpc_conn_read_response /home/runner/work/lightning/lightning/plugins/libplugin.c:1366:9
    #6 0x557da5adbea5 in next_plan /home/runner/work/lightning/lightning/ccan/ccan/io/io.c:60:9
    #7 0x557da5ae06ff in do_plan /home/runner/work/lightning/lightning/ccan/ccan/io/io.c:422:8
    ElementsProject#8 0x557da5adfb58 in io_ready /home/runner/work/lightning/lightning/ccan/ccan/io/io.c:439:10
    ElementsProject#9 0x557da5aec2ce in io_loop /home/runner/work/lightning/lightning/ccan/ccan/io/poll.c:455:5
    ElementsProject#10 0x557da59757ac in plugin_main /home/runner/work/lightning/lightning/plugins/libplugin.c:2409:3
    ElementsProject#11 0x557da594fe23 in main /home/runner/work/lightning/lightning/plugins/funder.c:1723:2
    ElementsProject#12 0x7f6572229d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16

SUMMARY: AddressSanitizer: heap-buffer-overflow asan_interceptors.cpp.o in printf_common(void*, char const*, __va_list_tag*)
Shadow bytes around the buggy address:
  0x0c487fff9350: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c487fff9360: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c487fff9370: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c487fff9380: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c487fff9390: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c487fff93a0: 00 00 00 00 00[fa]fa fa fa fa fa fa fa fa fa fa
  0x0c487fff93b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c487fff93c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c487fff93d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c487fff93e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c487fff93f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==26122==ABORTING
```

Signed-off-by: Rusty Russell <[email protected]>
cdecker pushed a commit that referenced this pull request Nov 27, 2025
@rustyrussell
fuzz-tests: Add a wire test for wireaddr functions
a59d21d 
Changelog-None: `towire_wireaddr()` and `fromwire_wireaddr()` in
`common/wireaddr.h` are responsible for marshalling/unmarshalling
BOLT #7 address descriptors.

Since these aren't tested by the existing wire fuzz tests, add a
roundtrip test for them. This has the added benefit of testing
`parse_wireaddr()` as well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@s373nZ