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

mempool: Rename RelayNonStd config option. #1024

Merged
merged 2 commits into from
Feb 15, 2018

Conversation

dnldd
Copy link
Member

@dnldd dnldd commented Feb 5, 2018

This PR contains the following upstream commits:

  • 26e2279
    • extended the commit by renaming RelayNonStdTxs to AcceptNonStdTxs for network params, renaming RelayNonStd to AcceptNonStd for dcrd's config struct and renaming relaynonstd config param to acceptnonstd. The sample config and doc entries were also updated accordingly.

This renames the mempool.Config.RelayNonStd option to AcceptNonStd which
more accurately describes its behavior since the mempool was refactored
into a separate package.

The reasoning for this change is that the mempool is not responsible for
relaying transactions (nor should it be).  Its job is to maintain a pool
of unmined transactions that are validated according to consensus and
policy configuration options which are then used to provide a source of
transactions that need to be mined.

Instead, it is the server that is responsible for relaying transactions.
While it is true that the current server code currently only relays txns
that were accepted to the mempool, this does not necessarily have to
be the case.  It would be entirely possible (and perhaps even a good
idea as something do in the future), to separate the relay policy from
the mempool acceptance policy (and thus indirectly the mining policy).
@davecgh
Copy link
Member

davecgh commented Feb 14, 2018

I'm not sure I completely agree with the renaming here. It no longer conveys that those transactions will be relayed to the rest of the network, which is important. That said, if the comments and descriptions are updated accordingly, I think it's acceptable.

This renames the mempool.Config.RelayNonStd option to AcceptNonStd which
more accurately describes its behavior since the mempool was refactored
into a separate package.

The reasoning for this change is that the mempool is not responsible for
relaying transactions (nor should it be).  Its job is to maintain a pool
of unmined transactions that are validated according to consensus and
policy configuration options which are then used to provide a source of
transactions that need to be mined.

Instead, it is the server that is responsible for relaying transactions.
While it is true that the current server code currently only relays txns
that were accepted to the mempool, this does not necessarily have to
be the case.  It would be entirely possible (and perhaps even a good
idea as something do in the future), to separate the relay policy from
the mempool acceptance policy (and thus indirectly the mining policy).
@dnldd dnldd force-pushed the merge_mempool_rename_relaynonstd branch from 47d57bb to e48b9ab Compare February 14, 2018 15:56
@davecgh davecgh merged commit e48b9ab into decred:master Feb 15, 2018
@dnldd dnldd deleted the merge_mempool_rename_relaynonstd branch June 4, 2018 23:41
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