This is the output of a (on-going) review of darkfi’s implementation of Crypsinous to understand whether we can reuse their code for Nomos. Everybody is welcomThe following subsections explain how this conclusion could be made and exact numbers for μμe to add thoughts or feedback

Code style

The code seems to be written by a C programmer without too much attention to Rust idioms.

Examples:

• the identifier of a fork is a i64 (https://github.com/darkrenaissance/darkfi/blob/e5e105ca40e538ebed7dc8981084f0b01db5601a/src/consensus/validator.rs#L354)
• -1 is the special magic number used to refer to the canonical chain throughout the code (https://github.com/darkrenaissance/darkfi/blob/e5e105ca40e538ebed7dc8981084f0b01db5601a/src/consensus/validator.rs#L388C1-L388C1).
• -2 looks like a special magic number too: https://github.com/darkrenaissance/darkfi/blob/e5e105ca40e538ebed7dc8981084f0b01db5601a/src/consensus/validator.rs#L745C14-L745C14
• https://github.com/darkrenaissance/darkfi/blob/0231beba7f3a3059efed9fdc825eb10d5c1a3d49/src/consensus/state.rs#L460 and https://github.com/darkrenaissance/darkfi/blob/0231beba7f3a3059efed9fdc825eb10d5c1a3d49/src/consensus/state.rs#L477 could have been written more easily as iterator
• retain should have been used instead https://github.com/darkrenaissance/darkfi/blob/e5e105ca40e538ebed7dc8981084f0b01db5601a/src/consensus/validator.rs#L448
• thread sleep on async code: https://github.com/darkrenaissance/darkfi/blob/0231beba7f3a3059efed9fdc825eb10d5c1a3d49/src/consensus/clock.rs#L160

Software design / architecture

• Validator actions should not depend on whether it’s a leader or not https://github.com/darkrenaissance/darkfi/blob/e5e105ca40e538ebed7dc8981084f0b01db5601a/src/consensus/validator.rs#L673
• https://github.com/darkrenaissance/darkfi/blob/e5e105ca40e538ebed7dc8981084f0b01db5601a/src/consensus/validator.rs#L788 Checks that transactions on a fork are valid. Invalid blocks should be discarded upon reception and a valid block can’t become invalid so there should be no need to do this. Shows non optimal organization of data flow
• state is flag based?: https://github.com/darkrenaissance/darkfi/blob/e5e105ca40e538ebed7dc8981084f0b01db5601a/src/consensus/state.rs#L70

Correctness

• https://github.com/darkrenaissance/darkfi/blob/0231beba7f3a3059efed9fdc825eb10d5c1a3d49/src/consensus/validator.rs#L717 What is chain finalization?