Hold reconfig lock while handling transaction #19320
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Skipped Deployments
|
|
Can you explain more? I don't fully understand how this could crash and why the PR fixes it |
|
Added some more explanation to the description |
|
Looks like there is still compilation error? |
indeed, thought it was a simple enough change to oneshot it but i was wrong |
mystenmark
force-pushed
the
mlogan-handle-tx-reconfig-lock-guard
branch
from
2370952 to
da6d526
Compare
mystenmark
force-pushed
the
mlogan-handle-tx-reconfig-lock-guard
branch
from
da6d526 to
bcbd41a
Compare
Oh, I used the wrong lock. Should work now. |
crates/sui-core/src/authority.rs
Outdated
| } | ||
|
|
||
| pub async fn execution_lock_for_signing(&self) -> ExecutionLockReadGuard { | ||
| // No need to check epoch here - current epoch is always correct for signing |
There was a problem hiding this comment.
Could you comment explain why signing also requires execution lock?
There was a problem hiding this comment.
Did you read the PR description? It's to prevent reconfiguration from happening while we are signing a tx.
There was a problem hiding this comment.
Yes, but please also doc comment it.
There was a problem hiding this comment.
oh of course, I have no idea why i thought you meant "comment in the PR"
crates/sui-core/src/authority.rs
Outdated
| return Err(SuiError::ValidatorHaltedAtEpochEnd); | ||
| } | ||
| // Ensure that validator cannot reconfigure while we are signing the tx | ||
| let _execution_lock = self.execution_lock_for_signing().await; |
There was a problem hiding this comment.
can this lock be moved into handle_transaction_impl so it also works with #19401
This fixes a crash where we clear all pending locks at reconfig (https://github.com/MystenLabs/sui/blob/dec33d0a303a7f83e778fcc5a136ab9776162e68/crates/sui-core/src/execution_cache/object_locks.rs#L121) while trying to acquire locks for a transaction.
mystenmark
force-pushed
the
mlogan-handle-tx-reconfig-lock-guard
branch
from
bcbd41a to
a643b6c
Compare
4 participants
This fixes a crash where we clear all pending locks at reconfig
(
sui/crates/sui-core/src/execution_cache/object_locks.rs
Line 121 in dec33d0
while trying to acquire locks for a transaction.
Without this fix, the node can reconfigure while we are trying to acquire locks for a transaction. If
clear_locks()linked above is called while we are trying to callclear_cached_locksatsui/crates/sui-core/src/execution_cache/object_locks.rs
Line 245 in 74d6d56
sui/crates/sui-core/src/execution_cache/object_locks.rs
Line 160 in 74d6d56