Witness flag erroneously activated on PoS networks #2720
Comments
Hmmm ok I will have a look at this, we are upgrading protocol version for PH so it might cause ripple effects, but we should actually not enable witness unless its activated |
Looking into this in more detail, the reason why the version is incorrectly set is actually due to the following:
It looks like this affects all inbound peers via the following:
I believe the transaction options causing witness requirements are still coming from the default setting in |
Hi @zeptin, can you trace "version" payloads and check that it is indeed the case that we send version as 70012? |
@bokobza Note that the protocol version actually gets used in several different payloads. But yes, it does.
|
(The above are some incoming connections from testnet. They all sent |
Examining this further, there appears to currently be no impact having this flag set. The The supported flags get assigned via the following in the
While the flag should be correctly set depending on network and deployment status, this does not appear to be critical at this point. |
Probably the fix here should be to separate the services and make it per network (same like the refactor coming for NetwworkProtocol enum). |
In the constructor of
NetworkPeerConnectionParameters
:This seems to mean that, regardless of the protocol version defined in
Program.cs
forStratisD
(70000), when handshaking with a peer we bump it up toPROTOCOL_VERSION
(70012) and enable the witness flag in the transaction options.I am not entirely sure if there are adverse consequences for this being enabled. but it does mean that we incorrectly say we want blocks with witness data when constructing
getdata
payloads (e.g. in the block puller), for instance:The text was updated successfully, but these errors were encountered: