Skip to content

Commit

Permalink
Check dependent packages exist before publishing
Browse files Browse the repository at this point in the history
  • Loading branch information
@lxfind
lxfind committed Jan 31, 2022
1 parent 945fa41 commit 4f3de32
Show file tree
Hide file tree
Showing 5 changed files with 250 additions and 107 deletions.
137 changes: 75 additions & 62 deletions fastpay_core/src/authority.rs
View file
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
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
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

0 comments on commit 4f3de32

Please sign in to comment.