We have another option here. We could break the API (revert), release 0.32.10 then yank 0.32.9.

We have another option here. We could break the API (revert), release 0.32.10 then yank 0.32.9.

I personally think this would indeed be preferable, for the reasons I wrote here:

However, here is why I think that a revert would be acceptable in this case:

• It's 0.32.9 that made a breaking change in the contract of VarInt. The fact that the code still compiles, to me, makes that worse: in the crate I'm maintaining, it was only discovered in the CI. Had there be no tests covering it, it could very easily reach a release and only be discovered with a production panic. In fact, that could still happen on software wallets depending on the ledger_bitcoin_client if they update rust-bitcoin to 0.32.9 but not ledger_bitcoin_client.
• Reverting the commit restores to the behavior of 0.32.8, so there is no breaking change unless you're relying on the behavior introduced in 0.32.9. Therefore, I think that not compiling is in fact better in this case, precisely to avoid the kind of runtime bugs that a backwards-compatible fix might cause.

@apoelstra whats your thoughts on this? If we want to revert and re-release I'm inclined to think that we should create a 0.32.10 that is only 0.32.9 and this patch. Then release it and yank 0.32.9. Logiistically that would mean either rebasing 0.32.x (force pushing to public branch is a but racey though) or creating another branch to release 0.32.10 from. Do we merge this patch into 0.32.x branch in its current state (ie on top of whats already unreleased on that branch) and then continue releasing from this branch later on?

No, yanking only makes sense if there is a security issue or a bug in a particular version of a crate. It does not give us license to release API-breaking changes in a point release. Only a serious security issue would let us do that.

Fixed and fixed.

I think this is going to break a fuzz test somewhere between 0.32.x and master but I'm not exactly sure what or where the fix goes. Just flagging it so we don't forget.

EDIT: Oh no it won't because we don't fuzz types from 0.32 that do not exist on master, VarInt being one such type.

No, yanking only makes sense if there is a security issue or a bug in a particular version of a crate. It does not give us license to release API-breaking changes in a point release. Only a serious security issue would let us do that.

I don't really care but I thought we were accepting that we introduced a bug in 0.32.9 and this PR fixes that bug but because the fix is a breaking change we are leaving ugly as hell code in there. So it seems reasonable to yank 0.32.9 and re-release with just this patch (but without the ugliness in it). Or am I on crack?

but because the fix is a breaking change we are leaving ugly as hell code in there.

Yes.

So it seems reasonable to yank 0.32.9 and re-release with just this patch (but without the ugliness in it).

No, removing the ugliness would be a breaking change.

Yanking a version or not is a completely separate question from whether we should release breaking changes in a minor release.

I'm not sure exactly where the miscommunication is or confusion.

1. 0.32.8
2. 0.32.9 - Introduces Foo
3. 0.32.10 - Removes Foo (breaking as of 0.32.9 but if that is yanked then non-breaking as of 0.32.8)

It's still breaking wrt 0.32.9. Yanking 0.32.9 doesn't change that. It just prevents cargo users from adding a new dependency on 0.32.9.

Oh thanks, I misunderstood yanking. I didn't realize that yank did not remove it from the registry.

This is good to go then. Force push is rebase only.

If we'd caught this within an hour or two of publishing, I'd be fine treating yank as "removes from the registry".

But at 2+ weeks, especially when this latest release contained a bunch of bugfixes, I'd expect it to be pretty widely deployed (though would anybody be using the API that we'd be breaking? kinda doubtful honestly).