[authority] Refactor shared object authority logic #727
Conversation
| if !transaction.contains_shared_object() { | ||
| // TODO: Maybe add a warning here, no respectable authority should | ||
| // have sequenced this. | ||
| return Ok(()); | ||
| } |
There was a problem hiding this comment.
Not sure. A bad authority may input whatever it likes into consensus (because we want to use consensus as a black box).
There was a problem hiding this comment.
Not sure. A bad authority may input whatever it likes into consensus (because we want to use consensus as a black box).
@gdanezis
There was a problem hiding this comment.
Yes, but at this point the consensus may be giving us a hint about which authority sequenced a non-shared object transaction, and we could log this as a byzantine error?
sui_core/src/authority.rs
Outdated
| match (lock, object) { | ||
| (Some(seq), Some(object)) => { | ||
| if object.version() != seq { | ||
| warn!(object_version =? object.version(), |
There was a problem hiding this comment.
Are we sure we want to print 'warn' when the users do something bad? Shouldn't it be 'debug'?
There was a problem hiding this comment.
This is the same code as before. I have no feelings about the type of log to produce here. Maybe @velvia has a view about this?
There was a problem hiding this comment.
Yeah I wasn't sure of the level of logging needed as it seemed like this is something that shouldn't happen, maybe someone can shed a bit more light here?
| if !transaction.contains_shared_object() { | ||
| // TODO: Maybe add a warning here, no respectable authority should | ||
| // have sequenced this. | ||
| return Ok(()); | ||
| } |
There was a problem hiding this comment.
Not sure. A bad authority may input whatever it likes into consensus (because we want to use consensus as a black box).
@gdanezis
gdanezis
force-pushed
the
authority-refactor-shared
branch
from
36d9aaa to
8658396
Compare
| @@ -193,7 +193,10 @@ impl AuthorityState { | |||
| } | |||
| }; | |||
| } | |||
| InputObjectKind::SharedMoveObject(..) => (), | |||
| InputObjectKind::SharedMoveObject(..) => { | |||
There was a problem hiding this comment.
@asonnino I added this check. However this treats immutable & shared objects uniformly, and that worries me.
| } | ||
|
|
||
| /// Read a lock for a specific (transaction, shared object) pair. | ||
| pub fn all_shared_locks( |
There was a problem hiding this comment.
@alberto This is how you get a sequence using the first element of a compound index.
Refactored the authority logic:
multi-getwhen possible to read locks and objects.A few questions for @asonnino below.