Upgrade trader accounts to Taproot/MuSig2
Depends on https://github.com/lightninglabs/pool/pull/370.
Pull Request Checklist
- [X] LndServices minimum version has been updated if new lnd apis/fields are used.
I addressed all comments and got almost all itests working on the server side. This is now officially not WIP anymore :tada:
@positiveblue: review reminder
I addressed all comments except for the additional unit tests. I agree they should be added, but I'm going to work on them next week.
might need to extend unit test frame work
I added a set of unit tests for the new spend paths. And I also ran the fuzzer locally for almost 2 hours and didn't find anything so far.
Ready for deployment modulo ~~when the subasta PR is merged.~~ Do not deploy until then
Requirements for merge:
- deploy the server side
- gated by lnd 0.15.1 to certify that bitcoind has taproot availability
⚠️ PR will need to go back to sprint as changes have been made to MuSig2 spec regarding x-only keys
Deliverable outcome:
- [ ] Modify the implemented MuSig2 spec to incoporate recent changes which will promote ISP
warning PR will need to go back to sprint as changes have been made to MuSig2 spec regarding x-only keys
Deliverable outcome:
* [ ] Modify the implemented MuSig2 spec to incoporate recent changes which will promote ISP
Are we sure we want to wait for that? Updating to the latest spec means:
- Implement new API in
btcd, merge. - Implement changes to MuSig2 API in
lnd, merge, release - Use new API in Pool.
I think it would make more sense to instead add a version parameter to the MuSig2 API when we implement this, so we can upgrade later.
PR will need to go back to sprint as changes have been made to MuSig2 spec regarding x-only keys
No, it can proceed as defined. As mentioned, those changes themselves are still in a draft state, with no other implementations currently tracking the change (other than the python ref implementation used to generate the test vectors). Correctness is still retained without the changes. It would matter more in a p2p setting as you need cross implementation compatibility.
Ofc, we could delay things, but then what if we hit another change in serialization somewhere that is "better" but doesn't affect correctness. Would we then do the same and pause to target the newer change? The tradeoff of being one of the first in the ecosystem to utilize these new features is that we sort of need to commit to having the proper versioning to handle future updates.
@guggero agreed, iiuc we already have the versioning in the proper places as well. An account version is the most apt, since the serialization changes will change the actual aggregate key, so making the jump a future version would be an actual account version bump.
Agree that versioning should be incorporated here to facilitate it being upgraded. Appears there was some misinterpretation about the code etc constrained/delayed by MuSig2 spec, which won't be the plan.