Abstract out some of the descriptor Span-parsing helpers #16887
Merged
+252
−57
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
ACK c072446b5c7fbaabcd17494d1b593b1cb5ebc314, verified it's mostly moved code - new code is just forward declarations, license and include guards. |
Contributor
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
laanwj
reviewed
Sep 18, 2019
Member
|
Concept ACK. Now that these functions are public, would be nice to have unit tests. |
sipa
force-pushed
the
201909_spanparsing
branch
from
c072446 to
5e69aee
Compare
Contributor
Member
Author
|
@theStack Be my guest! |
Contributor
Unit tests for the Span-parsing helpers are ready, will open a PR for them as soon as this one is merged in (I guess it's not possible to open a PR which depends on another PR, right?) |
Member
Author
|
@theStack You can post a commit, and I can include it in my PR. You can also open a PR against my branch. |
Member
👍 Then we can have the tests alongside the changes in the same PR. @theStack obviously your changes remain attributed to you. |
Contributor
|
@sipa @fanquake Okay, here we go: theStack@f4db52f |
Member
Member
Member
Author
|
Will do next week. |
tests the following four functions:
- Const() [parse constant]
- Func() [parse function]
- Expr() [parse expression]
- Split() [split up a string]
Member
Author
|
Thanks, @theStack; I've included your commit. |
Contributor
|
The parsing heavy functions |
Member
|
ACK bb36372 Show signature and timestampSignature: Timestamp of file with hash |
Member
|
Pretty straightforward move-only commit and then a commit adding tests, going to merge now. |
maflcko
pushed a commit
that referenced
this pull request
Oct 10, 2019
bb36372 test: add unit tests for Span-parsing helpers (Sebastian Falbesoner) 5e69aee Add documenting comments to spanparsing.h (Pieter Wuille) 230d43f Abstract out some of the descriptor Span-parsing helpers (Pieter Wuille) Pull request description: As suggested here: #16800 (comment). This moves the Span parsing functions out of the descriptor module, making them more easily usable for other parsers (in particular, in preparation for miniscript parsing). ACKs for top commit: MarcoFalke: ACK bb36372 Tree-SHA512: b5c5c11a9bc3f0a1c2c4cfa22755654ecfb8d4b69da0dc1fb9f04e1556dc0f6ffd87ad153600963279ac465d587d7971b53d240ced802d12693682411ac73deb
Contributor
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Oct 11, 2019
… helpers bb36372 test: add unit tests for Span-parsing helpers (Sebastian Falbesoner) 5e69aee Add documenting comments to spanparsing.h (Pieter Wuille) 230d43f Abstract out some of the descriptor Span-parsing helpers (Pieter Wuille) Pull request description: As suggested here: bitcoin#16800 (comment). This moves the Span parsing functions out of the descriptor module, making them more easily usable for other parsers (in particular, in preparation for miniscript parsing). ACKs for top commit: MarcoFalke: ACK bb36372 Tree-SHA512: b5c5c11a9bc3f0a1c2c4cfa22755654ecfb8d4b69da0dc1fb9f04e1556dc0f6ffd87ad153600963279ac465d587d7971b53d240ced802d12693682411ac73deb
maflcko
pushed a commit
that referenced
this pull request
Oct 16, 2019
…helpers 58d67f1 tests: Add fuzzing harness for descriptor Span-parsing helpers (practicalswift) Pull request description: Add fuzzing harness for descriptor Span-parsing helpers (`spanparsing`). As suggested by a fuzz testing enthusiast in #16887 (comment). **Testing this PR** Run: ``` $ CC=clang CXX=clang++ ./configure --enable-fuzz \ --with-sanitizers=address,fuzzer,undefined $ make $ src/test/fuzz/spanparsing ``` ACKs for top commit: MarcoFalke: re-ACK 58d67f1 Tree-SHA512: 5eaca9fcda2856e0dcfeb4a98a2dc97051ae6251f7642b92fdae3ff96bb95ccb0377ee4e6c6b531e59061983b8d9485a5282467f2ab1d614861f60202a893b1c
deadalnix
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Jul 9, 2020
… helpers Summary: bitcoin/bitcoin@230d43f --- Partial backport of Core [[bitcoin/bitcoin#16887 | PR16887]] Test Plan: ninja check Reviewers: #bitcoin_abc, deadalnix Reviewed By: #bitcoin_abc, deadalnix Subscribers: deadalnix Differential Revision: https://reviews.bitcoinabc.org/D6877
jasonbcox
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Jul 9, 2020
Summary: bitcoin/bitcoin@5e69aee --- Depends on D6877 Partial backport of Core [[bitcoin/bitcoin#16887 | PR16887]] Test Plan: read the comments Reviewers: #bitcoin_abc, deadalnix Reviewed By: #bitcoin_abc, deadalnix Subscribers: deadalnix Differential Revision: https://reviews.bitcoinabc.org/D6878
jasonbcox
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Jul 9, 2020
Summary:
tests the following four functions:
- Const() [parse constant]
- Func() [parse function]
- Expr() [parse expression]
- Split() [split up a string]
bitcoin/bitcoin@bb36372
---
Depends on D6878
Concludes backport of Core [[bitcoin/bitcoin#16887 | PR16887]]
Test Plan:
ninja check-bitcoin-util_tests
Reviewers: #bitcoin_abc, deadalnix
Reviewed By: #bitcoin_abc, deadalnix
Differential Revision: https://reviews.bitcoinabc.org/D6879
jasonbcox
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Jul 9, 2020
…ng helpers Summary: 58d67f1cc068c3779e309dc8a82ce33158c3e5b2 tests: Add fuzzing harness for descriptor Span-parsing helpers (practicalswift) Pull request description: Add fuzzing harness for descriptor Span-parsing helpers (`spanparsing`). As suggested by a fuzz testing enthusiast in bitcoin/bitcoin#16887 (comment). **Testing this PR** Run: ``` $ CC=clang CXX=clang++ ./configure --enable-fuzz \ --with-sanitizers=address,fuzzer,undefined $ make $ src/test/fuzz/spanparsing ``` --- Depends on D6879 Backport of Core [[bitcoin/bitcoin#17113 | PR17113]] Test Plan: cmake -GNinja .. -DENABLE_SANITIZERS="address;fuzzer" -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ ninja bitcoin-fuzzers link-fuzz-test_runner.py run `./src/test/fuzz/spanparsing` for a few seconds Reviewers: #bitcoin_abc, deadalnix Reviewed By: #bitcoin_abc, deadalnix Differential Revision: https://reviews.bitcoinabc.org/D6880
7 tasks
kwvg
added a commit
to kwvg/dash
that referenced
this pull request
Oct 29, 2021
pravblockc
pushed a commit
to pravblockc/dash
that referenced
this pull request
Nov 18, 2021
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
8 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As suggested here: #16800 (comment).
This moves the Span parsing functions out of the descriptor module, making them more easily usable for other parsers (in particular, in preparation for miniscript parsing).