namecoin / namecoin-core

Namecoin full node + wallet based on the current Bitcoin Core codebase.
https://www.namecoin.org/
MIT License
455 stars 146 forks source link

Fixing versionbits #384

Open sidhujag opened 3 years ago

sidhujag commented 3 years ago

Hello,

Syscoin is auxpow just like Namecoin. I have fixed the versionbits to work with AuxPow you may refer to my implementation if you would like, it will require a Namecoin fork and we also have to lobby for Bitcoin to never use bit 8 which I think they are aware of. In my implementation chainid has no value unless the auxpow bit (bit 8) is set, bitcoin is free to use any other bits unlike the existing implementation which will error if bitcoin uses bits representing the chainid(bits 16 to around 27), so I think its better to let them use those aslong as bit 8 isn't flagged denoting auxpow. Here is the latest commit on it with working code (functional tests, unit tests, CI): https://github.com/syscoin/syscoin/commit/150346b980a21475badacd50ffdec5a4f5028a80

domob1812 commented 3 years ago

Thanks for your suggestion! We've been thinking about this for a long time now, and even had a half-way working implementation in the past. I think our proposal was/is to just get rid of the auxpow bit and make every block auxpow instead (which is what makes most sense for miners anyway).

This is similar to what we did with Xaya.

sidhujag commented 3 years ago

Thanks for your suggestion! We've been thinking about this for a long time now, and even had a half-way working implementation in the past. I think our proposal was/is to just get rid of the auxpow bit and make every block auxpow instead (which is what makes most sense for miners anyway).

This is similar to what we did with Xaya.

hmm remove auxpow bit is interesting but would be a mess with functional bitcoin tests I think? But I do think there is merit because right now there is assumption that bitcoin doesn't use bit 16 -> 24ish for majority auxpow coins, for nmc its just bit 16 and 17 I believe because otherwise the consensus validation will fail during parent chain id checks. If you remove the bit thats fine but I still thought chainid needs to be encoded? The hard part isn't auxpow bit but more chainid encoding along with bitcoin playing nicely on it. In my implementation it only matters if the parent is flagging auxpow anyway so its probably fine, but in all current implementations out there that assumption is not there, if btc uses chainid bits it will mess up where I simply ensure that auxpow is set then return chainid otherwise chainid isn't there.

domob1812 commented 3 years ago

I think the chain ID is not an issue. What we can do is just change the serialisation format of the auxpow to include the chain ID as well. In other words, implement a fork that does something like this:

  1. When a block's timestamp is below X, apply old behaviour.
  2. When a block's timestamp is beyond X, assume the block is auxpow (independent of nVersion) and also use a new format for deserialising the auxpow part, namely including the chain ID somewhere.

Although I believe the chain ID is indeed not strictly necessary to be encoded. There is still a chain ID conceptually, but it only plays into how the auxpow is validated. There is no need to encode it explicitly into the block as far as I can tell.

There is some documentation about how we are doing something along these lines for Xaya, if you are interested.

A long time ago we wanted to do something like this for Namecoin, but I think it died down at some point because we were not sure if doing a hardfork was a good idea at all at the time. IMHO it would be very useful to revive these efforts at some point.

sidhujag commented 3 years ago

I think the chain ID is not an issue. What we can do is just change the serialisation format of the auxpow to include the chain ID as well. In other words, implement a fork that does something like this:

  1. When a block's timestamp is below X, apply old behaviour.
  2. When a block's timestamp is beyond X, assume the block is auxpow (independent of nVersion) and also use a new format for deserialising the auxpow part, namely including the chain ID somewhere.

Although I believe the chain ID is indeed not strictly necessary to be encoded. There is still a chain ID conceptually, but it only plays into how the auxpow is validated. There is no need to encode it explicitly into the block as far as I can tell.

There is some documentation about how we are doing something along these lines for Xaya, if you are interested.

A long time ago we wanted to do something like this for Namecoin, but I think it died down at some point because we were not sure if doing a hardfork was a good idea at all at the time. IMHO it would be very useful to revive these efforts at some point.

where would it get included? Certainly breaking parsing behaviour is undesired. In Syscoin chain ID was a problem as we used 4096 which ended up using dummy bit 28. We are now going to use bit 8, but it is a problem if bitcoin decides to use bit16/17 for NMC even though they do not flag bit 8 (auxpow), in my work I ensure bit 8 is set in order to check for chain ID. So afaics right now bitcoin wouldn't be able to either use bit16/17 or bit 8 otherwise parent chain ID check will fail, in my code now its fine to reuse those bits because they only need to not flag bit 8 (auxpow). We do need to check for parent auxpow in case someone tries to reuse work on existing chain ID or another auxpow coin that is merged.

domob1812 commented 3 years ago

If you are doing a hard fork anyway (which a change like this would be), there's nothing wrong with changing the parsing format. So you can easily add the chain ID to the auxpow data structure if you want. (But as described, I don't think this is even necessary, as the chain ID will be implicit anyway.)

sidhujag commented 3 years ago

If you are doing a hard fork anyway (which a change like this would be), there's nothing wrong with changing the parsing format. So you can easily add the chain ID to the auxpow data structure if you want. (But as described, I don't think this is even necessary, as the chain ID will be implicit anyway.)

Yea I can see putting it into auxpow making sense. This would require a spec update and any auxpow parsers I guess. I think that may be a fair tradeoff.

What do you mean it is implicit chain ID? If it is not part of the header somewhere how would you know if the parent is mining your block is following the same policies that is being enforced via chain ID checks right now?

domob1812 commented 3 years ago

True, any external software parsing auxpow's would need to be updated. I'm not really aware of any, though. And everything that would be affected will be easy to fix.

If it is not part of the header somewhere how would you know if the parent is mining your block is following the same policies that is being enforced via chain ID checks right now?

All you need is proof that someone committed hash power towards your block; I don't think it matters in any way to have the parent chain explicitly state the chain ID it was mining. You still need the chain ID hardcoded in your daemon, though (and in theory also in the miner), to find the right spot in the chain Merkle tree. I just don't think you have to explicitly encode it into the serialised form.

But maybe I'm missing something here (I've not thought about it too deeply). Are you thinking of a specific issue / attack vector that would be possible if the chain ID is not explicitly coded into the serialised block?

sidhujag commented 3 years ago

True, any external software parsing auxpow's would need to be updated. I'm not really aware of any, though. And everything that would be affected will be easy to fix.

If it is not part of the header somewhere how would you know if the parent is mining your block is following the same policies that is being enforced via chain ID checks right now?

All you need is proof that someone committed hash power towards your block; I don't think it matters in any way to have the parent chain explicitly state the chain ID it was mining. You still need the chain ID hardcoded in your daemon, though (and in theory also in the miner), to find the right spot in the chain Merkle tree. I just don't think you have to explicitly encode it into the serialised form.

But maybe I'm missing something here (I've not thought about it too deeply). Are you thinking of a specific issue / attack vector that would be possible if the chain ID is not explicitly coded into the serialised block?

Yea true, I always thought it was there to avoid some fringe case of someone trying to re-use work of a childchain or something. The immediate thing that came to mind was somehow using work as a child acting as parent of subsequent blocks but I am not sure if it matters as you say the difficulty algorithm still ensures work is done correctly it shouldn't matter where or who did the work aslong as it passes the difficulty challenge. The reason I thought that was because there are explicit checks that the parent and child chain ID cannot match.

https://github.com/namecoin/namecoin-core/blob/master/src/auxpow.cpp#L44

domob1812 commented 3 years ago

Ah yes, that check. I can imagine it is there to prevent directly mining a block and using it as auxpow for the next one (so getting two for the price of one), as you say. But in practice, that is unlikely to happen, since it is far more profitable to mine a Bitcoin block instead of a Namecoin one. Also, if we were to require that all blocks have an auxpow and none are directly mined, this problem would not be there either.