Skip to content

Commit

Permalink
[objects] authority can return object layout to client in ObjectInfoR…
Browse files Browse the repository at this point in the history
…equest

More progress on interpreting object bytes inside the client.

- Modify the authority's ObjectInfoRequest to allow the client to (optionally) request the `MoveStructLayout` for an object in a variety of formats
- Modify the authority's ObjectInfoResponse to return the layout if requested
- Implement the appropriate traits in the authority to allow computing the layout from its local DB

Note: three approaches to this were discussed in MystenLabs#397 and there was a general consensus that the third one (ask clients to store packages + use this to compute a layout locally) was preferred. However, after looking into this a bit, I concluded that this will be a major change to the client, which is not great because we need to start piping out JSON objects into the client service API's ASAP to unblock Geniteam (ideally today!).

In addition to the prioritization, I think approach (3) doesn't help a client who wants to get a structured view of an object that is not owned by them. Asking such a user to download every module related that object just to look at it + go through the dance of computing a layout from these modules doesn't seem great. Thus, I think exposing a way to ask an authority for the layout is reasonable even if we also implement approach (3).
  • Loading branch information
sblackshear committed Feb 11, 2022
1 parent d71aceb commit b0e4fc9
Show file tree
Hide file tree
Showing 7 changed files with 89 additions and 8 deletions.
1 change: 1 addition & 0 deletions sui_core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ sui-network = { path = "../network_utils" }
sui-types = { path = "../sui_types" }

move-binary-format = { git = "https://github.com/diem/move", rev="1e5e36cb1fede8073c8688886a0943bb5bfe40d5" }
move-bytecode-utils = { git = "https://github.com/diem/move", rev="1e5e36cb1fede8073c8688886a0943bb5bfe40d5" }
move-core-types = { git = "https://github.com/diem/move", rev="1e5e36cb1fede8073c8688886a0943bb5bfe40d5", features=["address20"] }
move-package = { git = "https://github.com/diem/move", rev="1e5e36cb1fede8073c8688886a0943bb5bfe40d5" }
move-vm-runtime = { git = "https://github.com/diem/move", rev="1e5e36cb1fede8073c8688886a0943bb5bfe40d5" }
Expand Down
22 changes: 21 additions & 1 deletion sui_core/src/authority.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Facebook, Inc. and its affiliates.
// SPDX-License-Identifier: Apache-2.0

use move_bytecode_utils::module_cache::ModuleCache;
use move_core_types::{
account_address::AccountAddress,
language_storage::{ModuleId, StructTag},
Expand Down Expand Up @@ -439,8 +440,19 @@ impl AuthorityState {
} else {
self.get_order_lock(&object.to_object_reference()).await?
};
let layout = match request.request_layout {
Some(format) => {
let resolver = ModuleCache::new(&self);
object.get_layout(format, &resolver)?
}
None => None,
};

Some(ObjectResponse { object, lock })
Some(ObjectResponse {
object,
lock,
layout,
})
}
Err(e) => return Err(e),
_ => None,
Expand Down Expand Up @@ -630,3 +642,11 @@ impl AuthorityState {
Ok(filtered)
}
}

impl ModuleResolver for AuthorityState {
type Error = SuiError;

fn get_module(&self, module_id: &ModuleId) -> Result<Option<Vec<u8>>, Self::Error> {
self._database.get_module(module_id)
}
}
19 changes: 19 additions & 0 deletions sui_core/src/authority/authority_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -451,3 +451,22 @@ impl AuthorityStore {
}))
}
}

impl ModuleResolver for AuthorityStore {
type Error = SuiError;

fn get_module(&self, module_id: &ModuleId) -> Result<Option<Vec<u8>>, Self::Error> {
match self.get_object(module_id.address())? {
Some(o) => match &o.data {
Data::Package(c) => Ok(c
.get(module_id.name().as_str())
.cloned()
.map(|m| m.into_vec())),
_ => Err(SuiError::BadObjectType {
error: "Expected module object".to_string(),
}),
},
None => Ok(None),
}
}
}
16 changes: 12 additions & 4 deletions sui_core/src/authority_aggregator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -427,11 +427,17 @@ where
// None if the object was deleted at this authority
//
// NOTE: here we could also be gathering the locked orders to see if we could make a cert.
let (object_option, signed_order_option) =
if let Some(ObjectResponse { object, lock }) = object_and_lock {
(Some(object), lock)
// TODO: pass along layout_option so the client can store it
let (object_option, signed_order_option, _layout_option) =
if let Some(ObjectResponse {
object,
lock,
layout,
}) = object_and_lock
{
(Some(object), lock, layout)
} else {
(None, None)
(None, None, None)
};

// Update the map with the information from this authority
Expand Down Expand Up @@ -1072,6 +1078,8 @@ where
let request = ObjectInfoRequest {
object_id,
request_sequence_number: None,
// TODO: allow caller to specify layout
request_layout: None,
};

// For now assume all authorities. Assume they're all honest
Expand Down
11 changes: 9 additions & 2 deletions sui_types/src/messages.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) Facebook, Inc. and its affiliates.
// SPDX-License-Identifier: Apache-2.0

use crate::object::{Object, OBJECT_START_VERSION};
use crate::object::{Object, ObjectFormatOptions, OBJECT_START_VERSION};

use super::{base_types::*, committee::Committee, error::*, event::Event};

Expand All @@ -10,7 +10,7 @@ use super::{base_types::*, committee::Committee, error::*, event::Event};
mod messages_tests;

use move_binary_format::{access::ModuleAccess, CompiledModule};
use move_core_types::{identifier::Identifier, language_storage::TypeTag};
use move_core_types::{identifier::Identifier, language_storage::TypeTag, value::MoveStructLayout};
use serde::{Deserialize, Serialize};
use std::{
collections::{BTreeSet, HashSet},
Expand Down Expand Up @@ -124,13 +124,16 @@ pub struct ObjectInfoRequest {
pub object_id: ObjectID,
/// The version of the object for which the parent certificate is sought.
pub request_sequence_number: Option<SequenceNumber>,
/// If true, the request will return the layout of the object in the given format
pub request_layout: Option<ObjectFormatOptions>,
}

impl From<ObjectRef> for ObjectInfoRequest {
fn from(object_ref: ObjectRef) -> Self {
ObjectInfoRequest {
object_id: object_ref.0,
request_sequence_number: Some(object_ref.1),
request_layout: None,
}
}
}
Expand All @@ -140,6 +143,7 @@ impl From<ObjectID> for ObjectInfoRequest {
ObjectInfoRequest {
object_id,
request_sequence_number: None,
request_layout: None,
}
}
}
Expand All @@ -157,6 +161,9 @@ pub struct ObjectResponse {
/// Order the object is locked on in this authority.
/// None if the object is not currently locked by this authority.
pub lock: Option<SignedOrder>,
/// Schema of the Move value inside this object.
/// None if the object is a Move package, or the request did not ask for the layout
pub layout: Option<MoveStructLayout>,
}

/// This message provides information about the latest object and its lock
Expand Down
25 changes: 24 additions & 1 deletion sui_types/src/object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ const ID_END_INDEX: usize = AccountAddress::LENGTH;
/// Index marking the end of the object's version + the beginning of type-specific data
const VERSION_END_INDEX: usize = ID_END_INDEX + 8;

/// Different schemes for converting a Move object to JSON
/// Different schemes for converting a Move value into a structured representation
#[derive(Eq, PartialEq, Debug, Clone, Deserialize, Serialize, Hash)]
pub struct ObjectFormatOptions {
/// If true, include the type of each object as well as its fields; e.g.:
/// `{ "fields": { "f": 20, "g": { "fields" { "h": true }, "type": "0x0::MyModule::MyNestedType" }, "type": "0x0::MyModule::MyType" }`
Expand Down Expand Up @@ -400,6 +401,20 @@ impl Object {
}
}

/// Get a `MoveStructLayout` for `self`.
/// The `resolver` value must contain the module that declares `self.type_` and the (transitive)
/// dependencies of `self.type_` in order for this to succeed. Failure will result in an `ObjectSerializationError`
pub fn get_layout(
&self,
format: ObjectFormatOptions,
resolver: &impl GetModule,
) -> Result<Option<MoveStructLayout>, SuiError> {
match &self.data {
Data::Move(m) => Ok(Some(m.get_layout(format, resolver)?)),
Data::Package(_) => Ok(None),
}
}

/// Convert `self` to the JSON representation dictated by `format`.
/// If `self` is a Move value, the `resolver` value must contain the module that declares `self.type_` and the (transitive)
/// dependencies of `self.type_` in order for this to succeed. Failure will result in an `ObjectSerializationError`
Expand Down Expand Up @@ -465,3 +480,11 @@ impl Display for Object {
)
}
}

impl Default for ObjectFormatOptions {
fn default() -> Self {
ObjectFormatOptions {
include_types: true,
}
}
}
3 changes: 3 additions & 0 deletions sui_types/src/unit_tests/serialize_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,12 @@ fn test_info_request() {
let req1 = ObjectInfoRequest {
object_id: dbg_object_id(0x20),
request_sequence_number: None,
request_layout: None,
};
let req2 = ObjectInfoRequest {
object_id: dbg_object_id(0x20),
request_sequence_number: Some(SequenceNumber::from(129)),
request_layout: None,
};

let buf1 = serialize_object_info_request(&req1);
Expand Down Expand Up @@ -244,6 +246,7 @@ fn test_info_response() {
object_and_lock: Some(ObjectResponse {
object: object.clone(),
lock: Some(vote),
layout: None,
}),
parent_certificate: None,
requested_object_reference: Some(object.to_object_reference()),
Expand Down

0 comments on commit b0e4fc9

Please sign in to comment.