Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add dependent package objects to input_objects during module publish #300

Merged
merged 1 commit into from
Jan 31, 2022
+250 −107
Merged

Add dependent package objects to input_objects during module publish #300

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
137 changes: 75 additions & 62 deletions fastpay_core/src/authority.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,63 +61,75 @@ impl AuthorityState {
fn check_one_lock(
&self,
order: &Order,
object_ref: ObjectRef,
object: Object,
) -> Result<(ObjectRef, Object), FastPayError> {
let (object_id, sequence_number, object_digest) = object_ref;

fp_ensure!(
sequence_number <= SequenceNumber::max(),
FastPayError::InvalidSequenceNumber
);

// Check that the seq number is the same
fp_ensure!(
object.version() == sequence_number,
FastPayError::UnexpectedSequenceNumber {
object_id,
expected_sequence: object.version(),
object_kind: InputObjectKind,
object: &Object,
) -> FastPayResult {
match object_kind {
InputObjectKind::MovePackage(package_id) => {
fp_ensure!(
object.data.try_as_package().is_some(),
FastPayError::MoveObjectAsPackage {
object_id: package_id
}
);
}
);
InputObjectKind::MoveObject((object_id, sequence_number, object_digest)) => {
fp_ensure!(
sequence_number <= SequenceNumber::max(),
FastPayError::InvalidSequenceNumber
);

// Check that the seq number is the same
fp_ensure!(
object.version() == sequence_number,
FastPayError::UnexpectedSequenceNumber {
object_id,
expected_sequence: object.version(),
}
);

// Check the digest matches
fp_ensure!(
object.digest() == object_digest,
FastPayError::InvalidObjectDigest {
object_id,
expected_digest: object_digest
}
);

// Check the digest matches
fp_ensure!(
object.digest() == object_digest,
FastPayError::InvalidObjectDigest {
object_id,
expected_digest: object_digest
}
);

if object.is_read_only() {
// Gas object must not be immutable.
fp_ensure!(
&object_id != order.gas_payment_object_id(),
FastPayError::InsufficientGas {
error: "Gas object should not be immutable".to_string()
if object.is_read_only() {
// Gas object must not be immutable.
fp_ensure!(
&object_id != order.gas_payment_object_id(),
FastPayError::InsufficientGas {
error: "Gas object should not be immutable".to_string()
}
);
// Checks for read-only objects end here.
return Ok(());
}
);
// Checks for read-only objects end here.
return Ok((object_ref, object));
}

// Additional checks for mutable objects
// Check the transaction sender is also the object owner
fp_ensure!(
order.sender() == &object.owner,
FastPayError::IncorrectSigner
);

if &object_id == order.gas_payment_object_id() {
gas::check_gas_requirement(order, &object)?;
}
// Additional checks for mutable objects
// Check the transaction sender is also the object owner
fp_ensure!(
order.sender() == &object.owner,
FastPayError::IncorrectSigner
);

Ok((object_ref, object))
if &object_id == order.gas_payment_object_id() {
gas::check_gas_requirement(order, object)?;
}
}
};
Ok(())
}

/// Check all the objects used in the order against the database, and ensure
/// that they are all the correct version and number.
async fn check_locks(&self, order: &Order) -> Result<Vec<(ObjectRef, Object)>, FastPayError> {
async fn check_locks(
&self,
order: &Order,
) -> Result<Vec<(InputObjectKind, Object)>, FastPayError> {
let input_objects = order.input_objects();
let mut all_objects = Vec::with_capacity(input_objects.len());

Expand All @@ -128,27 +140,25 @@ impl AuthorityState {
);
// Ensure that there are no duplicate inputs
let mut used = HashSet::new();
if !input_objects.iter().all(|o| used.insert(o)) {
if !input_objects.iter().all(|o| used.insert(o.object_id())) {
return Err(FastPayError::DuplicateObjectRefInput);
}

let ids: Vec<_> = input_objects.iter().map(|(id, _, _)| *id).collect();
let ids: Vec<_> = input_objects.iter().map(|kind| kind.object_id()).collect();

let objects = self.get_objects(&ids[..]).await?;
let mut errors = Vec::new();
for (object_ref, object) in input_objects.into_iter().zip(objects) {
for (object_kind, object) in input_objects.into_iter().zip(objects) {
let object = match object {
Some(object) => object,
None => {
errors.push(FastPayError::ObjectNotFound {
object_id: object_ref.0,
});
errors.push(object_kind.object_not_found_error());
continue;
}
};

match self.check_one_lock(order, object_ref, object) {
Ok((object_ref, object)) => all_objects.push((object_ref, object)),
match self.check_one_lock(order, object_kind, &object) {
Ok(()) => all_objects.push((object_kind, object)),
Err(e) => {
errors.push(e);
}
Expand Down Expand Up @@ -179,11 +189,14 @@ impl AuthorityState {
.check_locks(&order)
.await?
.into_iter()
.filter_map(|(object_ref, object)| {
if object.is_read_only() {
None
} else {
Some(object_ref)
.filter_map(|(object_kind, object)| match object_kind {
InputObjectKind::MovePackage(_) => None,
InputObjectKind::MoveObject(object_ref) => {
if object.is_read_only() {
None
} else {
Some(object_ref)
}
}
})
.collect();
Expand Down Expand Up @@ -219,7 +232,7 @@ impl AuthorityState {
.check_locks(&order)
.await?
.into_iter()
.map(|(_object_ref, object)| object)
.map(|(_, object)| object)
.collect();

// Insert into the certificates map
Expand Down
74 changes: 38 additions & 36 deletions fastpay_core/src/client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,12 +450,12 @@ where
// Put back the target cert
missing_certificates.push(target_cert);

for object_ref in input_objects {
for object_kind in input_objects {
// Request the parent certificate from the authority.
let object_info_response = source_client
.handle_object_info_request(ObjectInfoRequest {
object_id: object_ref.0,
request_sequence_number: Some(object_ref.1),
object_id: object_kind.object_id(),
request_sequence_number: Some(object_kind.version()),
request_received_transfers_excluding_first_nth: None,
})
.await;
Expand Down Expand Up @@ -706,17 +706,13 @@ where
sender: FastPayAddress,
order: Order,
) -> Result<(Vec<(CertifiedOrder, OrderInfoResponse)>, CertifiedOrder), anyhow::Error> {
let mut input_objects = Vec::new();
for (object_id, seq, _) in &order.input_objects() {
input_objects.push((*object_id, *seq));
}

for (object_id, target_sequence_number, _) in &order.input_objects() {
let next_sequence_number = self.next_sequence_number(object_id).unwrap_or_default();
for object_kind in &order.input_objects() {
let object_id = object_kind.object_id();
let next_sequence_number = self.next_sequence_number(&object_id).unwrap_or_default();
fp_ensure!(
target_sequence_number >= &next_sequence_number,
object_kind.version() >= next_sequence_number,
FastPayError::UnexpectedSequenceNumber {
object_id: *object_id,
object_id,
expected_sequence: next_sequence_number,
}
.into()
Expand Down Expand Up @@ -770,7 +766,7 @@ where
async fn broadcast_and_execute<'a, V, F: 'a>(
&'a mut self,
sender: FastPayAddress,
inputs: Vec<ObjectRef>,
inputs: Vec<InputObjectKind>,
certificates_to_broadcast: Vec<CertifiedOrder>,
action: F,
) -> Result<(Vec<(CertifiedOrder, OrderInfoResponse)>, Vec<V>), anyhow::Error>
Expand All @@ -784,14 +780,15 @@ where
Some(sender),
);

let known_certificates = inputs.iter().flat_map(|(object_id, seq, _)| {
self.certificates(object_id).filter_map(move |cert| {
if cert.order.sender() == &sender {
Some(((*object_id, *seq), Ok(cert)))
} else {
None
}
})
let known_certificates = inputs.iter().flat_map(|input_kind| {
self.certificates(&input_kind.object_id())
.filter_map(move |cert| {
if cert.order.sender() == &sender {
Some(((input_kind.object_id(), input_kind.version()), Ok(cert)))
} else {
None
}
})
});

let (_, mut handle) = Downloader::start(requester, known_certificates);
Expand All @@ -805,7 +802,9 @@ where
// Figure out which certificates this authority is missing.
let mut responses = Vec::new();
let mut missing_certificates = Vec::new();
for (object_id, target_sequence_number, _) in inputs {
for input_kind in inputs {
let object_id = input_kind.object_id();
let target_sequence_number = input_kind.version();
let request = ObjectInfoRequest {
object_id,
request_sequence_number: None,
Expand Down Expand Up @@ -864,7 +863,7 @@ where
async fn broadcast_confirmation_orders(
&mut self,
sender: FastPayAddress,
inputs: Vec<ObjectRef>,
inputs: Vec<InputObjectKind>,
certificates_to_broadcast: Vec<CertifiedOrder>,
) -> Result<Vec<(CertifiedOrder, OrderInfoResponse)>, anyhow::Error> {
self.broadcast_and_execute(sender, inputs, certificates_to_broadcast, |_, _| {
Expand All @@ -885,7 +884,13 @@ where
let known_sequence_numbers: BTreeSet<_> = self
.certificates(&object_id)
.flat_map(|cert| cert.order.input_objects())
.filter_map(|(id, seq, _)| if id == object_id { Some(seq) } else { None })
.filter_map(|object_kind| {
if object_kind.object_id() == object_id {
Some(object_kind.version())
} else {
None
}
})
.collect();

let mut requester = CertificateRequester::new(
Expand Down Expand Up @@ -922,16 +927,13 @@ where
.order
.input_objects()
.iter()
.find_map(
|(id, seq, _)| {
if object_id == id {
Some(seq)
} else {
None
}
},
)
.cloned()
.find_map(|object_kind| {
if object_id == &object_kind.object_id() {
Some(object_kind.version())
} else {
None
}
})
.unwrap_or_default();

let mut new_next_sequence_number = self.next_sequence_number(object_id)?;
Expand Down Expand Up @@ -1234,8 +1236,8 @@ where

// The new cert will not be updated by order effect without confirmation, the new unconfirmed cert need to be added temporally.
let new_sent_certificates = vec![new_certificate.clone()];
for (object_id, _, _) in new_certificate.order.input_objects() {
self.update_certificates(&object_id, &new_sent_certificates)?;
for object_kind in new_certificate.order.input_objects() {
self.update_certificates(&object_kind.object_id(), &new_sent_certificates)?;
}

Ok(new_certificate)
Expand Down
52 changes: 52 additions & 0 deletions fastpay_core/src/unit_tests/authority_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,58 @@ async fn test_publish_module_no_dependencies_ok() {
)
}

#[tokio::test]
async fn test_publish_non_existing_dependent_module() {
let (sender, sender_key) = get_key_pair();
let gas_payment_object_id = ObjectID::random();
let gas_payment_object = Object::with_id_owner_for_testing(gas_payment_object_id, sender);
let gas_payment_object_ref = gas_payment_object.to_object_reference();
// create a genesis state that contains the gas object and genesis modules
let (genesis_module_objects, _) = genesis::clone_genesis_data();
let genesis_module = match &genesis_module_objects[0].data {
Data::Package(m) => CompiledModule::deserialize(m.values().next().unwrap()).unwrap(),
_ => unreachable!(),
};
// create a module that depends on a genesis module
let mut dependent_module = make_dependent_module(&genesis_module);
// Add another dependent module that points to a random address, hence does not exist on-chain.
dependent_module
.address_identifiers
.push(AccountAddress::random());
dependent_module.module_handles.push(ModuleHandle {
address: AddressIdentifierIndex((dependent_module.address_identifiers.len() - 1) as u16),
name: IdentifierIndex(0),
});
let dependent_module_bytes = {
let mut bytes = Vec::new();
dependent_module.serialize(&mut bytes).unwrap();
bytes
};
let authority = init_state_with_objects(vec![gas_payment_object]).await;

let order = Order::new_module(
sender,
gas_payment_object_ref,
vec![dependent_module_bytes],
&sender_key,
);

let response = authority.handle_order(order).await;
assert!(response
.unwrap_err()
.to_string()
.contains("DependentPackageNotFound"));
// Check that gas was not charged.
assert_eq!(
authority
.object_state(&gas_payment_object_id)
.await
.unwrap()
.version(),
gas_payment_object_ref.1
);
}

// Test the case when the gas provided is less than minimum requirement during module publish.
// Note that the case where gas is insufficient to publish the module is tested
// separately in the adapter tests.
Expand Down
Loading