kindelia / Kindelia

An efficient, secure cryptocomputer
https://kindelia.org/
609 stars 38 forks source link

refactor(err): remove some unwraps in node.rs #261

Closed dan-da closed 1 year ago

dan-da commented 1 year ago

changes:

Regarding Mutex::lock().... many rust crates/apps use the parking_lot crate instead which provides a faster Mutex that does not require an unwrap. So we could consider using it.

The unwrap on std::sync::Mutex is to handle the case where another thread has panic'ed and thus the locked data might be in an inconsistent state. parking_lot appears to just sort of ignore this case and hope for the best, which doesn't really seem correct... I'm not sure what their reasoning is... maybe valid? Since recovery of mutex poisoining might be difficult (not obvious how best to handle), I opted to leave the unwrap (panic) for now.

racs4 commented 1 year ago

Regarding Mutex::lock().... many rust crates/apps use the parking_lot crate instead which provides a faster Mutex that does not require an unwrap. So we could consider using it.

I don't know about this library, but I will research about it and about this subject. If you want you can also create an issue for this, so that anyone can give suggestions and remind us of that too (the todos in the code also help)

dan-da commented 1 year ago

ok, yeah I don't think Mutex should hold up review/merge of this PR, as I didn't change the mutex behavior other than unwrap() --> expect() for a better log message.

racs4 commented 1 year ago

There is a pending comment about self.get_block_info, I was just waiting for this to be answered, if you would plan on making this change in a future PR focused on node.rs

Apparently I made the comment and forgot to click in "Add Review", sorry