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

Additional fragments to help filtering spam transactions and reduce sync fails #857

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

theDavidCoen
Copy link

I added a couple of additional fragments to my BTCpay Server Docker because I had important sync issues for a few weeks that made my BTCpay basically useless.

These additional fragments attempts to reduce those sync problems by filtering some kind of transactions from the node's mempool.

opt-btc-permitbaremultisig.yml prevents your node to accept bare multisig transactions, which are non-common multisig transactions and are used today to bloat the UTXO set with uncompressed keys that are not real points on the secp256k1 curve.

opt-btc-datacarriersize.yml sets the datacarrier size to 83 byte, same as the default OP_RETURN size for Bitcoin Core, trying to filter extra data out of the node's mempool.

I propose to add them to the list of available additional fragments so the users can easily opt-in if they need.

Add a 83 byte limit to datacarriersize to match OP-RETURN default limit (in Bitcoin Core) and filter additional data.
Filter bare multisig transactions. 
Acts as a spam filter for non-common multisig transactions used to bloat the UTXO set.
Copy link
Member

@Kukks Kukks left a comment

Choose a reason for hiding this comment

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

concept ack, but should add them to the readme too

@dennisreimann
Copy link
Member

dennisreimann commented Dec 18, 2023

I'm not sure if we should add additional fragments per setting or rather simply document this. In the docs we could have a section that explains how to add custom options for bitcoind (like with lnd.conf) and whcih ones might make sense.

@theDavidCoen
Copy link
Author

@dennisreimann I'm not against simply documenting them, but since they are optional fragments and not active by default, it might be useful to add it to the available additional fragments.

However, it's really up to you guys.

@theDavidCoen
Copy link
Author

concept ack, but should add them to the readme too

@Kukks I never did that, sorry. Can you guide me through?

@theDavidCoen
Copy link
Author

…carriersize.yml

opt-btc-permitbaremultisig.yml and opt-btc-datacarriersize.yml added to README.md
Copy link
Author

@theDavidCoen theDavidCoen left a comment

Choose a reason for hiding this comment

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

I updated the README.md as well

Copy link
Member

@Kukks Kukks left a comment

Choose a reason for hiding this comment

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

Ack from my end, but maybe we need to see if it's realistically needed to have them as pre made options

@NicolasDorier
Copy link
Member

NicolasDorier commented Dec 19, 2023

set the datacarrier size parameter to 83 byte, same as the default OP_RETURN size for Bitcoin Core

What is the point them? we already use the default

permitbaremultisig=0

I doubt this has any impact

@theDavidCoen
Copy link
Author

theDavidCoen commented Dec 19, 2023

set the datacarrier size parameter to 83 byte, same as the default OP_RETURN size for Bitcoin Core

What is the point them? we already use the default

permitbaremultisig=0

I doubt this has any impact

@NicolasDorier you are right, I apologize.
This parameter does not enforce the limit on datacarring, but applies to OP_RETURN size, so it's redundant.

It would probably need this bitcoin/bitcoin#28408 to be effective.

I remove that fragment.

As for the second fragment, I noticed an increased number of fake public keys because of Stamps SRC20 protocol and https://mempool.space/ introduced a feature to monitor them as well. It could be useful if this number keeps going up.

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.

None yet

4 participants