Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactor the revocation signatures transmission to watchtowers #109

Closed
darosior opened this issue Oct 31, 2021 · 2 comments · Fixed by #115
Closed

Refactor the revocation signatures transmission to watchtowers #109

darosior opened this issue Oct 31, 2021 · 2 comments · Fixed by #115
Labels
RFC-settled This has been discussed on a higher level, and now likely needs spec update.

Comments

@darosior
Copy link
Member

darosior commented Oct 31, 2021 •
edited
Loading

Currently we'd share the signatures in 3 sig messages, 1 per transaction type. We leverage this in the implementation of the watchtower to avoid allocating fee-bumping reserves to vaults that are not delegated yet:

  1. Share the Emergency signature at revocation sigs exchange time to the watchtower
  2. Share the Cancel and UnvaultEmergency sigs at delegation time, wait ACK from watchtower before sharing Unvault sig with other participants

However, this is not applicable to watchtowers which don't store the Emergency signatures. It's also awkward for the wallet to sit on signatures and seems better to share them with the watchtower as soon as we can. We should instead have:

  1. A single revocation txs batch, the watchtower stores them but doesn't allocate reserves
  2. A request before delegating, if it can the watchtower allocates reserves and otherwise NACKs

EDIT: this fixes #98
@darosior darosior added the RFC This needs to be discussed at a higher level label Oct 31, 2021
@darosior darosior mentioned this issue Oct 31, 2021
Merged
darosior added a commit to revault/miradord that referenced this issue Oct 31, 2021
@darosior
Merge #17: listener: be less strict on duplicate Emergency signatures
8039cd6 
550df3c listener: be less strict on duplicate Emergency signatures (Antoine Poinsot)

Pull request description:

  This was neat, but in practice it's a pain. And i'd like to move to revault/practical-revault#109 ASAP anyways.

ACKs for top commit:
  darosior:
    ACK 550df3c -- trivial and i need it for the revaultd integration :)

Tree-SHA512: 1f1d4b2543aa48ef6405494c3959d3ce54137f7c7001267150f3dff5deac2a787b4501741d04262bdd2fe440b71b617293f0aa89776aa55e40b17a10cf235989
@darosior darosior mentioned this issue Nov 1, 2021
Merged
darosior added a commit to revault/revaultd that referenced this issue Nov 13, 2021
@darosior
Merge #308: Watchtower revocation signatures exchange
2270624 
81c532d ci: run misc and wt tests together (Antoine Poinsot)
f0d88bc qa: test basic spending policies on watchtowers (Antoine Poinsot)
e17247c rpc: share revoc signatures to watchtowers before sharing Unvault sigs (Antoine Poinsot)
7d48d3d tests: disable the watchtower for reorg tests (Antoine Poinsot)
1de4fee Share the Emergency signatures with the watchtower(s) (Antoine Poinsot)
94c336c tests: integrate the watchtower in the tests framework (Antoine Poinsot)
8aac1f5 tests: add miradord submodule (Antoine Poinsot)
8ecdc86 submodules: use 'master' branch (Antoine Poinsot)
5a0ff26 db: check for empty vector of txs in update_vault_status (Antoine Poinsot)

Pull request description:

  This pull request, based on #306, implements the distribution of revocation signatures to the watchtower (https://github.com/revault/miradord).

  The protocol defines a `sig` message per transaction and we leverage this on the watchtower to not allocate fee-bumping reserves before a vault is delegated. In order to do this we share the second-stage (ie the txs spending the Unvault) transactions signatures before sharing the Unvault to implicitly signal that we are about to share the Unvault transaction signature and the watchtower better be telling us not to do it now if it doesn't have sufficient reserve [0].
  Of course, this is all simulation as we don't have fee-bumping reserve management implemented on the watchtower yet.
  This translates here by having the background signature fetcher thread share the Emergency transaction signatures when it gathered them all (or to do it immediately in `revocationtxs` if we were passed them all at once).

  This also adds a few plugins to test some basic spending policies (revault everything, revault nothing, revault according to a maximum per day). Note that by default we set the "revault nothing" policies as by default the watchtower would currently revault everything and this would break most of our tests :).

  [0] I think we should not rely on this trick but instead modify the protocol. See revault/practical-revault#109

ACKs for top commit:
  danielabrozzoni:
    ACK 81c532d

Tree-SHA512: 2ab94a62d055d3ba6c9eb2d0e6d16e67e17e1f5e0b0c3b6bd1be27d9a182628a7414ac9282aff64962f52f9bb5b0a9f53ff1bd91dc05821fa04985dc24e34bbd
@darosior
Copy link
Member Author

darosior commented Jan 7, 2022

Conflicts with #98

@darosior
Copy link
Member Author

darosior commented Jan 7, 2022

So we wouldn't need 2. if we get rid of fee-bumping.

@darosior darosior mentioned this issue Jan 17, 2022
Merged
3 tasks
@darosior darosior added RFC-settled This has been discussed on a higher level, and now likely needs spec update. and removed RFC This needs to be discussed at a higher level labels Jan 17, 2022
@darosior darosior mentioned this issue Jan 18, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
RFC-settled This has been discussed on a higher level, and now likely needs spec update.
Projects
Revault project overview
Status: Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

messages: merge the sig messages to the WT into a single sigs darosior/practical-revault
1 participant
@darosior