Skip to content

Conversation

@theStack
Copy link
Contributor

@theStack theStack commented Aug 9, 2022

This PR renames the default in-mempool max ancestors/descendants constants MAX_ANCESTORS/MAX_DESCENDANTS in the functional tests to match the ones in the codebase:

/** Default for -limitancestorcount, max number of in-mempool ancestors */
static constexpr unsigned int DEFAULT_ANCESTOR_LIMIT{25};

/** Default for -limitdescendantcount, max number of in-mempool descendants */
static constexpr unsigned int DEFAULT_DESCENDANT_LIMIT{25};

The custom limit constants MAX_ANCESTORS_CUSTOM/MAX_DESCENDANTS_CUSTOM are also renamed accordingly to better fit to this naming style. In the second commit, the default constants are deduplicated by moving them into the messages.py module. (Not sure if this module is really appropriate, as it doesn't have a connection to messages. If someone has a good suggestion, would be glad to hear it.)

…_{ANCESTOR,DESCENDANT}_LIMIT`

-BEGIN VERIFY SCRIPT-
ren() { sed -i "s:$1:$2:g" $(git grep -l "$1" ./test); }

ren MAX_ANCESTORS_CUSTOM    CUSTOM_ANCESTOR_LIMIT
ren MAX_DESCENDANTS_CUSTOM  CUSTOM_DESCENDANT_LIMIT
ren MAX_ANCESTORS           DEFAULT_ANCESTOR_LIMIT
ren MAX_DESCENDANTS         DEFAULT_DESCENDANT_LIMIT
-END VERIFY SCRIPT-
@theStack theStack force-pushed the 202208-test-rename_default_ancestor_descendant_limits branch from ba53aaa to b4a5ab9 Compare August 9, 2022 13:23
@fanquake fanquake added the Tests label Aug 9, 2022
@fanquake fanquake requested a review from glozow August 9, 2022 13:31
@DrahtBot
Copy link
Contributor

DrahtBot commented Aug 9, 2022

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #25792 (test: add tests for datacarrier and datacarriersize options by w0xlt)

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.

Copy link
Contributor

@w0xlt w0xlt left a comment

Choose a reason for hiding this comment

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

ACK b4a5ab9

Copy link
Member

@glozow glozow left a comment

Choose a reason for hiding this comment

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

utACK b4a5ab9

@glozow
Copy link
Member

glozow commented Aug 10, 2022

In the second commit, the default constants are deduplicated by moving them into the messages.py module. (Not sure if this module is really appropriate, as it doesn't have a connection to messages. If someone has a good suggestion, would be glad to hear it.)

This seems fine given non-p2p-protocol constants like MAX_BIP125_RBF_SEQUENCE and COIN are there. Pinging to see if @MarcoFalke has some vision for organizing this kind of stuff.

Copy link
Member

@fanquake fanquake left a comment

Choose a reason for hiding this comment

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

ACK b4a5ab9

@maflcko maflcko merged commit 251c535 into bitcoin:master Aug 10, 2022
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Aug 11, 2022
@theStack theStack deleted the 202208-test-rename_default_ancestor_descendant_limits branch August 11, 2022 08:23
@bitcoin bitcoin locked and limited conversation to collaborators Aug 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants