Fix sequencer number bug (MystenLabs#1601)

* Fix bug MystenLabs#1600
* Shared object test

sui/tests/shared_objects_tests.rs

@@ -4,21 +4,24 @@ use bytes::Bytes;
 use futures::{sink::SinkExt, stream::StreamExt};
 use sui::config::AuthorityPrivateInfo;
 use sui_types::error::SuiError;
-use sui_types::messages::ConsensusTransaction;
-use sui_types::serialize::SerializedMessage;
-use sui_types::serialize::{deserialize_message, serialize_consensus_transaction};
+use sui_types::messages::CallArg;
+use sui_types::messages::Transaction;
+use sui_types::messages::TransactionInfoResponse;
+use sui_types::messages::{ConsensusTransaction, ExecutionStatus};
+use sui_types::serialize::{
+    deserialize_message, deserialize_transaction_info, serialize_consensus_transaction,
+};
+use sui_types::serialize::{serialize_cert, SerializedMessage};
 use test_utils::authority::{spawn_test_authorities, test_authority_configs};
-use test_utils::messages::test_shared_object_certificates;
+use test_utils::messages::{make_certificates, move_transaction, publish_move_package_transaction};
+use test_utils::messages::{parse_package_ref, test_shared_object_transactions};
 use test_utils::objects::{test_gas_objects, test_shared_object};
 use tokio::net::TcpStream;
 use tokio_util::codec::Framed;
 use tokio_util::codec::LengthDelimitedCodec;
 
-/// Submits a transaction to a Sui authority.
-async fn submit_transaction(
-    transaction: Bytes,
-    config: &AuthorityPrivateInfo,
-) -> SerializedMessage {
+/// Send bytes to a Sui authority.
+async fn transmit(transaction: Bytes, config: &AuthorityPrivateInfo) -> SerializedMessage {
     let authority_address = format!("{}:{}", config.host, config.port);
     let stream = TcpStream::connect(authority_address).await.unwrap();
     let mut connection = Framed::new(stream, LengthDelimitedCodec::new());
@@ -28,43 +31,157 @@ async fn submit_transaction(
     deserialize_message(&bytes[..]).unwrap()
 }
 
-// TODO: Taking too long to run. Re-enable once it's fixed.
-#[ignore]
-#[tokio::test]
-async fn shared_object_transaction() {
-    let mut objects = test_gas_objects();
-    objects.push(test_shared_object());
+/// Submit a certificate containing only owned-objects to all authorities.
+async fn submit_single_owner_transaction(
+    transaction: Transaction,
+    configs: &[AuthorityPrivateInfo],
+) -> Vec<TransactionInfoResponse> {
+    let certificate = make_certificates(vec![transaction]).pop().unwrap();
+    let serialized = Bytes::from(serialize_cert(&certificate));
 
-    // Get the authority configs and spawn them. Note that it is important to not drop
-    // the handles (or the authorities will stop).
-    let configs = test_authority_configs();
-    let _handles = spawn_test_authorities(objects, &configs).await;
+    let mut responses = Vec::new();
+    for config in configs {
+        let bytes = transmit(serialized.clone(), config).await;
+        let reply = deserialize_transaction_info(bytes).unwrap();
+        responses.push(reply);
+    }
+    responses
+}
 
-    // Make a test shared object certificate.
-    let certificate = test_shared_object_certificates().await.pop().unwrap();
+// Keep submitting the certificate until it is sequenced by consensus. We use the loop
+// since some consensus protocols (like Tusk) are not guaranteed to include the transaction
+// (but it has high probability to do so).
+// NOTE: This is good for testing but is not how a real client should submit transactions.
+async fn submit_shared_object_transaction(
+    transaction: Transaction,
+    configs: &[AuthorityPrivateInfo],
+) -> TransactionInfoResponse {
+    let certificate = make_certificates(vec![transaction]).pop().unwrap();
     let message = ConsensusTransaction::UserTransaction(certificate);
     let serialized = Bytes::from(serialize_consensus_transaction(&message));
 
-    // Keep submitting the certificate until it is sequenced by consensus. We use the loop
-    // since some consensus protocols (like Tusk) are not guaranteed to include the transaction
-    // (but it has high probability to do so).
-    tokio::task::yield_now().await;
     'main: loop {
-        for config in &configs {
-            match submit_transaction(serialized.clone(), config).await {
-                SerializedMessage::TransactionResp(_) => {
+        for config in configs {
+            match transmit(serialized.clone(), config).await {
+                SerializedMessage::TransactionResp(reply) => {
                     // We got a reply from the Sui authority.
-                    break 'main;
+                    break 'main *reply;
                 }
                 SerializedMessage::Error(error) => match *error {
                     SuiError::ConsensusConnectionBroken(_) => {
-                        // This is the (confusing) error message returned by the consensus adapter
-                        // timed out and didn't hear back from consensus.
+                        // This is the (confusing) error message returned by the consensus
+                        // adapter timed out and didn't hear back from consensus.
                     }
-                    error => panic!("Unexpected error {error}"),
+                    error => panic!("{error}"),
                 },
                 message => panic!("Unexpected protocol message {message:?}"),
             }
         }
     }
 }
+
+#[tokio::test]
+async fn shared_object_transaction() {
+    let mut objects = test_gas_objects();
+    objects.push(test_shared_object());
+
+    // Get the authority configs and spawn them. Note that it is important to not drop
+    // the handles (or the authorities will stop).
+    let configs = test_authority_configs();
+    let _handles = spawn_test_authorities(objects, &configs).await;
+
+    // Make a test shared object certificate.
+    let transaction = test_shared_object_transactions().pop().unwrap();
+
+    // Submit the transaction. Note that this transaction is random and we do not expect
+    // it to be successfully executed by the Move execution engine.
+    tokio::task::yield_now().await;
+    let reply = submit_shared_object_transaction(transaction, &configs).await;
+    assert!(reply.signed_effects.is_some());
+}
+
+#[tokio::test]
+async fn call_shared_object_contract() {
+    let mut gas_objects = test_gas_objects();
+
+    // Get the authority configs and spawn them. Note that it is important to not drop
+    // the handles (or the authorities will stop).
+    let configs = test_authority_configs();
+    let _handles = spawn_test_authorities(gas_objects.clone(), &configs).await;
+
+    // Publish the move package to all authorities and get the new pacakge ref.
+    tokio::task::yield_now().await;
+    let transaction = publish_move_package_transaction(gas_objects.pop().unwrap());
+    let replies = submit_single_owner_transaction(transaction, &configs).await;
+    let mut package_refs = Vec::new();
+    for reply in replies {
+        let effects = reply.signed_effects.unwrap().effects;
+        assert!(matches!(effects.status, ExecutionStatus::Success { .. }));
+        package_refs.push(parse_package_ref(&effects).unwrap());
+    }
+    let package_ref = package_refs.pop().unwrap();
+
+    // Make a transaction to create a counter.
+    tokio::task::yield_now().await;
+    let transaction = move_transaction(
+        gas_objects.pop().unwrap(),
+        "Counter",
+        "create",
+        package_ref,
+        /* arguments */ Vec::default(),
+    );
+    let replies = submit_single_owner_transaction(transaction, &configs).await;
+    let mut counter_ids = Vec::new();
+    for reply in replies {
+        let effects = reply.signed_effects.unwrap().effects;
+        assert!(matches!(effects.status, ExecutionStatus::Success { .. }));
+        let ((shared_object_id, _, _), _) = effects.created[0];
+        counter_ids.push(shared_object_id);
+    }
+    let counter_id = counter_ids.pop().unwrap();
+
+    // Ensure the value of the counter is `0`.
+    tokio::task::yield_now().await;
+    let transaction = move_transaction(
+        gas_objects.pop().unwrap(),
+        "Counter",
+        "assert_value",
+        package_ref,
+        vec![
+            CallArg::SharedObject(counter_id),
+            CallArg::Pure(0u64.to_le_bytes().to_vec()),
+        ],
+    );
+    let reply = submit_shared_object_transaction(transaction, &configs).await;
+    let effects = reply.signed_effects.unwrap().effects;
+    assert!(matches!(effects.status, ExecutionStatus::Success { .. }));
+
+    // Make a transaction to increment the counter.
+    tokio::task::yield_now().await;
+    let transaction = move_transaction(
+        gas_objects.pop().unwrap(),
+        "Counter",
+        "increment",
+        package_ref,
+        vec![CallArg::SharedObject(counter_id)],
+    );
+    let reply = submit_shared_object_transaction(transaction, &configs).await;
+    let effects = reply.signed_effects.unwrap().effects;
+    assert!(matches!(effects.status, ExecutionStatus::Success { .. }));
+
+    // Ensure the value of the counter is `1`.
+    tokio::task::yield_now().await;
+    let transaction = move_transaction(
+        gas_objects.pop().unwrap(),
+        "Counter",
+        "assert_value",
+        package_ref,
+        vec![
+            CallArg::SharedObject(counter_id),
+            CallArg::Pure(1u64.to_le_bytes().to_vec()),
+        ],
+    );
+    let reply = submit_shared_object_transaction(transaction, &configs).await;
+    let effects = reply.signed_effects.unwrap().effects;
+    assert!(matches!(effects.status, ExecutionStatus::Success { .. }));
+}

sui_core/src/authority/authority_store.rs

@@ -2,7 +2,6 @@
 // SPDX-License-Identifier: Apache-2.0
 use super::*;
 use crate::gateway_state::GatewayTxSeqNumber;
-
 use narwhal_executor::ExecutionIndices;
 use rocksdb::Options;
 use serde::{Deserialize, Serialize};
@@ -12,6 +11,7 @@ use std::path::Path;
 use sui_types::base_types::SequenceNumber;
 use sui_types::batch::{SignedBatch, TxSequenceNumber};
 use sui_types::crypto::{AuthoritySignInfo, EmptySignInfo};
+use sui_types::object::OBJECT_START_VERSION;
 use tracing::warn;
 use typed_store::rocks::{DBBatch, DBMap};
 
@@ -834,10 +834,13 @@ impl<const ALL_OBJ_VER: bool, S: Eq + Serialize + for<'de> Deserialize<'de>>
         let (sequenced_to_write, schedule_to_write): (Vec<_>, Vec<_>) = ids
             .zip(versions.iter())
             .map(|(id, v)| {
-                let version = v.unwrap_or_else(SequenceNumber::new);
+                // If it is the first time the shared object has been sequenced, assign it the default
+                // sequence number (`OBJECT_START_VERSION`). Otherwise use the `scheduled` map to
+                // to assign the next sequence number.
+                let version = v.unwrap_or_else(|| OBJECT_START_VERSION);
                 let next_version = v
                     .map(|v| v.increment())
-                    .unwrap_or_else(|| SequenceNumber::from(1));
+                    .unwrap_or_else(|| SequenceNumber::from(2));
 
                 let sequenced = ((transaction_digest, *id), version);
                 let scheduled = (id, next_version);

sui_core/src/unit_tests/authority_tests.rs

@@ -1414,7 +1414,7 @@ async fn shared_object() {
         use sui_types::gas_coin::GasCoin;
         use sui_types::object::MoveObject;
 
-        let content = GasCoin::new(shared_object_id, SequenceNumber::new(), 10);
+        let content = GasCoin::new(shared_object_id, OBJECT_START_VERSION, 10);
         let obj = MoveObject::new(/* type */ GasCoin::type_(), content.to_bcs_bytes());
         Object::new_move(obj, Owner::Shared, TransactionDigest::genesis())
     };
@@ -1479,7 +1479,7 @@ async fn shared_object() {
         .sequenced(&transaction_digest, [shared_object_id].iter())
         .unwrap()[0]
         .unwrap();
-    assert_eq!(shared_object_version, SequenceNumber::new());
+    assert_eq!(shared_object_version, OBJECT_START_VERSION);
 
     // Finally process the certificate and execute the contract. Ensure that the
     // shared object lock is cleaned up and that its sequence number increased.
@@ -1500,5 +1500,5 @@ async fn shared_object() {
         .unwrap()
         .unwrap()
         .version();
-    assert_eq!(shared_object_version, SequenceNumber::from(1));
+    assert_eq!(shared_object_version, SequenceNumber::from(2));
 }

sui_programmability/adapter/src/adapter.rs

@@ -801,7 +801,7 @@ pub fn resolve_and_type_check(
 }
 
 fn is_primitive(
-    module: &CompiledModule,
+    _module: &CompiledModule,
     function_type_arguments: &[TypeTag],
     t: &SignatureToken,
 ) -> bool {
@@ -812,7 +812,7 @@ fn is_primitive(
         | SignatureToken::U128
         | SignatureToken::Address => true,
 
-        SignatureToken::Vector(inner) => is_primitive(module, function_type_arguments, inner),
+        SignatureToken::Vector(inner) => is_primitive(_module, function_type_arguments, inner),
 
         SignatureToken::TypeParameter(idx) => function_type_arguments
             .get(*idx as usize)

sui_programmability/examples/basics/sources/Counter.move

@@ -45,6 +45,11 @@ module Basics::Counter {
         assert!(counter.owner == TxContext::sender(ctx), 0);
         counter.value = value;
     }
+
+    /// Assert a value for the counter.
+    public(script) fun assert_value(counter: &Counter, value: u64, _ctx: &mut TxContext) {
+        assert!(counter.value == value, 0)
+    }
 }
 
 #[test_only]

test_utils/Cargo.toml

@@ -19,6 +19,8 @@ tempfile = "3.3.0"
 bcs = "0.1.3"
 
 sui-adapter = { path = "../sui_programmability/adapter" }
+sui-framework = { path = "../sui_programmability/framework" }
+move-package = { git = "https://github.com/move-language/move", rev = "6a80792ecbf16d74bf1d57e48a576377f0879646" }
 move-core-types = { git = "https://github.com/move-language/move", rev = "6a80792ecbf16d74bf1d57e48a576377f0879646", features = ["address20"] }
 typed-store = { git = "https://github.com/MystenLabs/mysten-infra", rev ="d2976a45420147ad821baae96e6fe4b12215f743"}
 narwhal-config = { git = "https://github.com/MystenLabs/narwhal", rev = "8ae2164f0510349cbac2770e50e853bce5ab0e02", package = "config" }