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.

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

fix bug in enforcing nil-termination of new AGG_SIG_* conditions #270

Merged
merged 2 commits into from
Sep 22, 2023

Conversation

arvidn
Copy link
Contributor

@arvidn arvidn commented Sep 22, 2023

This fixes a bug when parsing the new AGG_SIG_* conditions in mempool mode.
The affected conditions are:

  1. AGG_SIG_PARENT
  2. AGG_SIG_PUZZLE
  3. AGG_SIG_AMOUNT
  4. AGG_SIG_PUZZLE_AMOUNT
  5. AGG_SIG_PARENT_PUZZLE
  6. AGG_SIG_PARENT_AMOUNT

In mempool mode, we require the exact correct number of arguments to be passed to conditions. Whereas in consensus mode we're more forgiving, to allow for soft-forks. The test for an NIL-terminator was incorrect and always looked at the last cons cell, not the last item in the list. This made this test fail unconditionally.

The bulk of this change is to extend the tests to cover this case.

The original feature landed here: #213

This PR is targeting a new branch called release-0.2.11 which was cut off of the most recent 0.2.10 release, to avoid pulling in any other changes for this release.

@@ -186,7 +186,7 @@ pub fn parse_args(
// AGG_SIG_* take two parameters

if (flags & STRICT_ARGS_COUNT) != 0 {
check_nil(a, c)?;
check_nil(a, rest(a, c)?)?;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is the bugfix. the nil-terminator is expected to be the next item in the list, not the last cons-cell.
The rest are extended tests to cover this and possible other differences in mempool-mode and consensus-mode

Copy link
Contributor

@richardkiss richardkiss left a comment

Choose a reason for hiding this comment

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

Looks good.

@arvidn arvidn merged commit ccdc34b into release-0.2.11 Sep 22, 2023
58 checks passed
@arvidn arvidn deleted the fix-new-agg-sig-parse-bug branch September 22, 2023 23:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants