revault / revault_tx

Script descriptors and transaction creation routines for Revault
BSD 3-Clause "New" or "Revised" License
15 stars 9 forks source link

scripts: limit the number of stakeholders and managers to 20 #123

Closed rndhouse closed 2 years ago

rndhouse commented 2 years ago

Issue: revault/practical-revault#110

WIP because I haven't ran the revaultd integration tests.

I don't understand why managers are included as stakeholders here (previously hitting stakeholder limit): https://github.com/revault/revault_tx/blob/c7e3e29ed37c06196ea241015cfe92c28d1957bf/src/scripts.rs#L1506

darosior commented 2 years ago

I don't understand why managers are included as stakeholders here (previously hitting stakeholder limit)

That looks like a pretty big oversight. And git blame says it's on me.. I've opened #124 to fix that. Thanks.

darosior commented 2 years ago

WIP because I haven't ran the revaultd integration tests.

Let me know once that's done. Current state of the PR looks good to me.

rndhouse commented 2 years ago

I used the this container to run the tests.

Non-server black box tests pass for this branch and revaultd. But note this commit: https://github.com/revault/revaultd/pull/389/commits/fcc2891babc2e781b5dda37d43450f4146755ce7

I get this error for server tests on this branch and for master (revaultd): https://github.com/revault/revaultd/pull/389#issuecomment-1104997911