Vendor 0.21.2

[tcharding]

This is #77 without the version bump patch.

Upgrade to a new way of vendoring Core and vendor version 0.21.2 - note please this will be directly followed by an upgrade to 0.21-final. Done separately to proof out the workflow of such an upgrade.

[apoelstra]

I'm gonna go ahead and review/ACK this although strictly speaking the intermediate commits don't compile. I think this crate has historically had a lot of vendoring-related grief and bisection is probably already pretty busted, and this puts us on the start to fixing that.

[tcharding]

Yes, my bad I could have mentioned that - they explicitly do not compile, and I had exactly the same thoughts as you mention. It would be nice if we could start getting them all to compile from now. The do in secp so I think we should, I"ll try and keep that as a goal.

[tcharding]

Force pushed the change discussed above, and tested with CORE_VENDOR_CP_NOT_CLONE=yes CORE_VENDOR_REPO=/home/tobin/build/github.com/tcharding/bitcoin contrib/vendor-bitcoin-core.sh v0.21.2

Thanks for you patience :)

[tcharding]

Now more like the secp script and includes -h option.

$ contrib/vendor-bitcoin-core.sh -h

Usage: script OPTIONS [bitcoin-core-version]

OPTIONS:

 -f    Vendor even if there are local changes to the rust-bitcoinconsensus git index
 -h    Print this help an exit

Example:

    vendor-bitcoin-core v0.21.2

[apoelstra]

Awesome :) we are super close. The vendoring script works now, but when I check the vendored code against the source I see

diff -r depend/bitcoin/src/clientversion.cpp depend2/bitcoin/src/clientversion.cpp
28c28
< #define GIT_COMMIT_ID "12d7ec38e2c69539d6d9d7e99c4da8e41b7824d6"
---
> #define GIT_COMMIT_ID "af591f2068d0363c92d9756ca39c43db85e5804c"

These two commit IDs are the tip of this PR in this repo and the commit from Bitcoin Core that we're vendoring.

This is suuper weird. If you look at bitcoin/src/clientversion.cpp you will find neither line. Instead you will see

  //! git will put "#define GIT_COMMIT_ID ..." on the next line inside archives. $Format:%n#define GIT_COMMIT_ID "%H"$

How does git do this and under what circumstances? I don't know. But I worry that it is putting our commit ID into this line rather than the Core commit ID. OTOH I don't think any of these constants are used by libbitcoinconsensus so maybe it doesn't matter..

Anyway I think I should just whitelist this specific file and forget about it. But let me know what you think.

[tcharding]

But I worry that it is putting our commit ID into this line rather than the Core commit ID.

I don't really like the idea of having this change in there, could bite us down the track. A simple hack could be, at the end of the vendoring script, to just grep for GIT_COMMIT_ID and echo a message "manually remove the change to GIT_COMMIT_ID" - we could spit out the old/new lines and the file.

[tcharding]

define GIT_COMMIT_ID "12d7ec38e2c69539d6d9d7e99c4da8e41b7824d6"

I had a bit of a play and we are still doing something different, I don't get these lines in clientversion.cpp, and they don't exist on this branch?

I tried using clone (and last time I pushed I was using copy)?

[apoelstra]

I think you need to use git archive or something to populate these lines. I am getting it by using Nix to source the git repo.

I think this is a non-issue because:

• The offending line doesn't actually appear in the repo; it is already dependent on some git magic that users are unlikely to trigger.
• The offending #define is not actually used in bitcoinconsensus anywhere.

[tcharding]

Cool, so this is good to go in then, right?

[apoelstra]

Yep. I'll tweak my scripts to ignore this line and ACK/merge it.