Adapt information about implemented UIPs and BIPs #895
Conversation
doc/uips.md
Outdated
| The unit-e code is based on the [Bitcoin Core | ||
| code](https://github.com/bitcoin/bitcoin). It inherits a part of its | ||
| implementation of | ||
| [BIPs](https://github.com/bitcoin/bitcoin/blob/0.16/doc/bips.md) from the last |
There was a problem hiding this comment.
I find it important to be able to track which BIPs are implemented in our protocol, why not just keeping bips.md and have it changed together with the Bitcoin version we are working with ? we're anyway referring here to the Bitcoin 0.16 BIPs, which gonna need to be changed to 0.17
There was a problem hiding this comment.
The reasoning here was to make it clear that the BIPs don't apply exactly as they are. They have to be read with a bit of interpretation because they contain references to Bitcoin, to specific block numbers, our block structure is a bit different, or because we don't have the Qt GUI. Not having the file in our repo, but referencing it with a note hopefully would make that clear.
The file also contains references to Bitcoin Core version numbers and pull requests which don't make much sense in the context of the unit-e repo.
We could keep the file and add a disclaimer at the top that this refers to Bitcoin Core and needs to be interpreted for unit-e.
Updating the link to the file with an upstream sync every six months should not be a big deal.
There was a problem hiding this comment.
in general we kinda inherit them with the code so whether we like it or not, other than quite annoying issues that they contain references to specific heights since BIPs are suppose to be client agnostic - which of them is implemented is quite an integral part of the codebase, we might even have many more BIPs inherited that UIPs generated by us.
sounds like we actually wanna curate / filter the bips.md, keeping the ones we actually have in the codebase, maybe commenting on ones that we implemented slightly different? except for the removed or enabled by default bips which are in this file, not sure how you can track it otherwise by just referring to Bitcoin Core bips.md
There was a problem hiding this comment.
I think ideally we would have something like an annotated list of BIPs which makes it clear how each BIP applies to unit-e. We have a part of that in the list of the BIPs which have been removed and activated by default. But we would need to go through the other BIPs as well and add comments where needed to make the list complete and accurate. We also would need to review this annotated list with each upstream sync and adapt it. Do you think it's something we should do now?
|
Why not have UIPs and BIPs? Some BIPs are literally implemented and do not affect bitcoin per se, but are for instance wallet designs. |
I think we should have UIPs and BIPs. I agree that it is important to document what BIPs are implemented. The question is how to do that from a practical point of view. Keeping the bips.md file as it comes from upstream appears to be confusing to me, because it has detailed references to Bitcoin Core, release versions, release dates, protocol bumps, block numbers, pull requests. That's why, in the first iteration of this pull request, I opted for removing it and referencing it from upstream with the comments about what we changed. So the information about both UIPs and BIPs is in one file. Alternatively we could keep the bips.md file, and add a comment at the top with the notes about what we changed. So having two files, and keeping the original upstream information in the file, not only referencing the file from the bitcoin repo. Or we actually write our own version of the list of BIPs with references to our own versions and comments where BIPs don't literally apply. That would probably be the nicest solution, but also quite a bit of effort. That's why I was hesitant to do that and opted for the simpler solution of just putting together the information we already have written down. What do you think would be the best solution for now? |
I've mentioned my thoughts above: "sounds like we actually wanna curate / filter the bips.md, keeping the ones we actually have in the codebase, maybe commenting on ones that we implemented slightly different? except for the removed or enabled by default bips which are in this file, not sure how you can track it otherwise by just referring to Bitcoin Core bips.md" so yeah I think we should keep bips.md with comments at the top should be good enough for now |
Ok, I'll adapt the pull request. |
* Add list of UIPs implemented in unit-e * Add information to the list of BIPs how they apply to unit-e Signed-off-by: Cornelius Schumacher <cornelius@thirdhash.com>
it with notes about changes in BIPs compared to the Bitcoin Core
version on which unit-e is based.
This fixes #640.
The information has been taken from https://github.com/dtr-org/unit-e-project/blob/master/bip-activation-reference.md, so that file can then be removed.