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.

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

Spin processors out into bespoke services #569

Closed
kayabaNerve opened this issue May 14, 2024 · 2 comments
Closed

Spin processors out into bespoke services #569

kayabaNerve opened this issue May 14, 2024 · 2 comments

Comments

@kayabaNerve
Copy link
Member

A single codebase to maintain and audit, an internal standard for interoperability, and a lack of duplicated code. This was the reasoning for writing the processor, singular.

To quote Douglas Adams,

This had made many people very angry and has been widely regarded as a bad move.

What really highlighted this is the Ethereum integration. It does not benefit from most of the fee code, due to using a relayer system (meaning Serai does not have to deduct the fee, solely calculate a fair fee rate). It does not benefit from the scheduling due to being in under the account model. It doesn't have branch/change addresses, as it has a constant external address deterministic to the first key generated.

It's all of this which effectively justifies a bespoke processor for Ethereum. While we currently are simply setting the fee to 0, causing the processor's code to effectively NOP, we now have to review every route and ensure it's actually NOP'ing as we do still call it (and simply expect a lack of effects).

Then, while working on the Ethereum integration, I realized: #470 (comment)

To quote directly here,

For Bitcoin specifically, we can claim infinite max outputs, take in all outputs in the Bitcoin integration, and then create the tree (signing every item in the tree immediately as one gigantic batch).

And it's with this, I finally have to accept the design philosophy of Godot.

Any universal API we try to create will be one we fight. While the current API is fine, and with the Scheduler modularity achieves our necessary goals, the optimal solution for any given integration will be specific to that integration. Accordingly, the sane thing is for the processor to become building blocks. A key gen service. A UTXO log scheduler. A SC-based account scheduler. A relayer fee system. A deducted fee system. Etc.

I won't claim these can't be re-composed back to a monolithic service at some point. I will claim I don't believe it's feasible to modularize the current processor so incrementally to the solutions we want. We'd at least have to go back to the drawing board, explicitly declare optimal flows, then declare modules, then fit the processor. Alternatively, we can move to bespoke solutions (removing the requirement of maintaining a perfectly generic monolith), and when we have the time, later recompose.

I say that while we also can't so redo the processor before mainnet. This will probably be kicked to #565 which is slated for after mainnet :/

@kayabaNerve
Copy link
Member Author

I ended up doing this now as the existing Processor was incapable of surviving an audit with any decent marks. The Ethereum integration was also horribly kludged in, with questions about how well it could actually be completed once token support was finished.

The processor-smash branch represents a processor built as it should have been, not one built, one which demonstrated what needed to be built, and then modified to be so built.

It has:

  • messages: The existing library defining Processor <-> Coordinator messages
  • frost-attempt-manager: A manager of attempts for the FROST signing protocol, which isn't actually Processor-specific and may end up also used in the coordinator
  • view-keys: A trivial library for generating view keys (as necessary to send to and audit Serai)
  • key-gen: The library for doing the eVRF DKG from One Round DKG #589, satisfying the API defined in messages
  • primitives: Primitive structures prior in networks/mod.rs
  • scanner: The new scanner, effectively replacing multisigs/ as it doesn't just scan blocks, yet also manages key rotation/the scheduler
  • scheduler: Various scheduler primitives and implementations
  • signers: The various signers (transaction, cosign, Batch, slash report), satisfying the API defined in messages
  • bitcoin, ethereum, monero: The new binaries

Of potential note is that the Completion API to terminate a signing protocol has been removed. Transaction signing protocols now only terminate when the signed transaction appears on-chain, or when the local processor completes the signature. Since we vote on re-attempts, the processors which signed the transaction will publish it and not cause a re-attempt. The other processors will keep the transaction in memory until it appears on-chain (which is fine). Removal of the optimistic path removes some complexity, and also should become fully redundant if we ever implement #588.

It is still a work in progress yet is about to wrap up.

@kayabaNerve
Copy link
Member Author

This resolved #284 #290 #296 #376 #509 #546 #550 #559, and that's not to mention all the TODOs present in the replaced code (nor its need to be cleaned up regardless, which this rewrite inherently does).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant