Potential thread panic from stack overflow error due to continuous recursive calls in `commit_block` in state.rs

[ghost]

The "commit_block" function inside state.rs has recursive calls to itself for committing parent blocks. However, this recursive approach may lead to stack overflow error if the depth of recursion becomes too large as it may exhaust all available memory allocated for the call stack.

/* ↓↓↓ For committing a block in insert_block ↓↓↓ */

/// Commits a block and its ancestors if they have not been committed already. If the commits cause
/// updates to the validator set, this returns them in sequence.
pub fn commit_block(
    &mut self,
    wb: &mut BlockTreeWriteBatch<K::WriteBatch>,
    block: &CryptoHash,
) -> Vec<ValidatorSetUpdates> {
    // ...
    // Recursion step:
       let mut validator_set_updates = if !block_justify.is_genesis_qc() {
           let parent = block_justify.block;
           self.commit_block(wb, &parent)
       } else {
           Vec::new()
       };

       // ...
       // Code snippet containing the commit_block function
}

Expected Behavior: The "commit_block" function should handle recursive calls without causing stack overflow error, ensuring proper completion of committing blocks.

Current state: There is no error handing to mitigate this even. Stack overflow error in "commit_block" function may result in thread panic. Thread panic due to stack overflow is not expected behaviour in production quality source code libraries and it brings down the reliability of this crate as an open source project.

Suggested Solution: To mitigate the risk of a stack overflow error, the recursive implementation of the "commit_block" function can be refactored to use an iterative approach or tail recursion.

If the authors are not in agreement, they are requested to prove that these calls are safe for usage.

[lyulka]

Thanks for opening this issue.

I like the simple, recursive definition of commit_block, but I acknowledge that preventing stack overflows is desirable.

We'll explore ways of keeping a simple definition of commit_block while preventing stack overflows as a future enhancement.

[ghost]

I get that you like the recursive definition of commit_block, but this isn't a pet project where you can implement anything that catches your fancy. What you personally like might not always be safe or appropriate to use. As an open source library, the goal should be to prioritize safety and reliability over individual/personal preferences.

So, make sure you're pulling your weight and not just pushing your own ideas without considering the safety implications of this design.