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.

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

[types] add ChainID to user txn #4969

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 10 additions & 0 deletions client/libra-dev/src/transaction.rs
Expand Up @@ -20,6 +20,7 @@ use libra_types::{
account_config::{
from_currency_code_string, lbr_type_tag, type_tag_for_currency_code, LBR_NAME,
},
chain_id::ChainId,
transaction::{
authenticator::AuthenticationKey, helpers::TransactionSigner, RawTransaction, Script,
SignedTransaction, TransactionArgument, TransactionPayload,
Expand All @@ -40,6 +41,7 @@ pub unsafe extern "C" fn libra_SignedTransactionBytes_from(
gas_unit_price: u64,
gas_identifier: *const i8,
expiration_time_secs: u64,
chain_id: u64,
script_bytes: *const u8,
script_len: usize,
ptr_buf: *mut *mut u8,
Expand Down Expand Up @@ -90,6 +92,7 @@ pub unsafe extern "C" fn libra_SignedTransactionBytes_from(
.to_string_lossy()
.into_owned(),
expiration_time,
ChainId::new(chain_id),
);

let keypair = KeyPair::from(private_key);
Expand Down Expand Up @@ -338,6 +341,7 @@ pub unsafe extern "C" fn libra_RawTransactionBytes_from(
sender: *const u8,
receiver: *const u8,
sequence: u64,
chain_id: u64,
num_coins: u64,
max_gas_amount: u64,
gas_unit_price: u64,
Expand Down Expand Up @@ -405,6 +409,7 @@ pub unsafe extern "C" fn libra_RawTransactionBytes_from(
gas_unit_price,
LBR_NAME.to_owned(),
expiration_time,
ChainId::new(chain_id),
);

let raw_txn_bytes = match to_bytes(&raw_txn) {
Expand Down Expand Up @@ -709,6 +714,7 @@ mod test {
gas_unit_price,
coin_ident.as_ptr(),
expiration_time_secs,
ChainId::test().id(),
script_bytes.as_ptr(),
script_len,
buf_ptr,
Expand Down Expand Up @@ -758,6 +764,7 @@ mod test {
gas_unit_price,
coin_idnet.as_ptr(),
expiration_time_secs,
ChainId::test().id(),
script_bytes.as_ptr(),
script_len,
buf_ptr2,
Expand Down Expand Up @@ -806,6 +813,7 @@ mod test {
gas_unit_price,
coin_ident_2.as_ptr(),
expiration_time_secs,
ChainId::test().id(),
script_bytes.as_ptr(),
script_len,
buf_ptr,
Expand Down Expand Up @@ -1046,6 +1054,7 @@ mod test {
sender_address.as_ref().as_ptr(),
receiver_address.as_ref().as_ptr(),
sequence,
ChainId::test().id(),
amount,
max_gas_amount,
gas_unit_price,
Expand Down Expand Up @@ -1140,6 +1149,7 @@ mod test {
gas_unit_price,
LBR_NAME.to_owned(),
Duration::from_secs(expiration_time_secs),
ChainId::test(),
),
public_key.clone(),
signature.clone(),
Expand Down
13 changes: 10 additions & 3 deletions client/swiss-knife/src/main.rs
Expand Up @@ -7,12 +7,16 @@ use libra_crypto::{
test_utils::KeyPair,
Signature, SigningKey, Uniform, ValidCryptoMaterialStringExt,
};
use libra_types::transaction::{
authenticator::AuthenticationKey, RawTransaction, SignedTransaction, Transaction,
TransactionPayload,
use libra_types::{
chain_id::ChainId,
transaction::{
authenticator::AuthenticationKey, RawTransaction, SignedTransaction, Transaction,
TransactionPayload,
},
};
use rand::{prelude::StdRng, SeedableRng};
use serde::{Deserialize, Serialize};
use std::str::FromStr;
use structopt::StructOpt;
use swiss_knife::helpers;

Expand Down Expand Up @@ -117,6 +121,8 @@ struct TxnParams {
pub sender_address: String,
// Sequence number of this transaction corresponding to sender's account.
pub sequence_number: u64,
// Chain ID of the network this transaction is intended for
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Chain ID of the network this transaction is intended for
// Chain ID of the Libra network this transaction is intended for

pub chain_id: String,
// Maximal total gas specified by wallet to spend for this transaction.
pub max_gas_amount: u64,
// Maximal price can be paid per gas.
Expand Down Expand Up @@ -193,6 +199,7 @@ fn generate_raw_txn(g: GenerateRawTxnRequest) -> GenerateRawTxnResponse {
g.txn_params.gas_unit_price,
g.txn_params.gas_currency_code,
std::time::Duration::new(g.txn_params.expiration_timestamp, 0),
ChainId::from_str(&g.txn_params.chain_id).expect("Failed to convert str to ChainId"),
);
GenerateRawTxnResponse {
raw_txn: hex::encode(
Expand Down
1 change: 1 addition & 0 deletions config/management/src/config_builder.rs
Expand Up @@ -108,6 +108,7 @@ impl<T: AsRef<Path>> ValidatorBuilder<T> {
validator_account,
validator_network_address,
fullnode_network_address,
self.template.base.chain_id,
&ns,
&ns_shared,
)
Expand Down
4 changes: 3 additions & 1 deletion config/management/src/lib.rs
Expand Up @@ -264,7 +264,7 @@ pub mod tests {
use super::*;
use crate::storage_helper::StorageHelper;
use libra_secure_storage::{CryptoStorage, KVStorage};
use libra_types::account_address::AccountAddress;
use libra_types::{account_address::AccountAddress, chain_id::ChainId};
use std::{
fs::File,
io::{Read, Write},
Expand Down Expand Up @@ -329,6 +329,7 @@ pub mod tests {
AccountAddress::random(),
"/ip4/0.0.0.0/tcp/6180".parse().unwrap(),
"/ip4/0.0.0.0/tcp/6180".parse().unwrap(),
ChainId::test(),
ns,
&((*ns).to_string() + shared),
)
Expand Down Expand Up @@ -394,6 +395,7 @@ pub mod tests {
AccountAddress::random(),
"/ip4/0.0.0.0/tcp/6180".parse().unwrap(),
"/ip4/0.0.0.0/tcp/6180".parse().unwrap(),
ChainId::test(),
local_ns,
remote_ns,
)
Expand Down
8 changes: 7 additions & 1 deletion config/management/src/storage_helper.rs
Expand Up @@ -11,7 +11,10 @@ use libra_network_address::NetworkAddress;
use libra_secure_storage::{
CryptoStorage, KVStorage, NamespacedStorage, OnDiskStorage, Storage, Value,
};
use libra_types::{account_address::AccountAddress, transaction::Transaction, waypoint::Waypoint};
use libra_types::{
account_address::AccountAddress, chain_id::ChainId, transaction::Transaction,
waypoint::Waypoint,
};
use std::{fs::File, path::Path};
use structopt::StructOpt;

Expand Down Expand Up @@ -214,6 +217,7 @@ impl StorageHelper {
owner_address: AccountAddress,
validator_address: NetworkAddress,
fullnode_address: NetworkAddress,
chain_id: ChainId,
local_ns: &str,
remote_ns: &str,
) -> Result<Transaction, Error> {
Expand All @@ -224,6 +228,7 @@ impl StorageHelper {
--owner-address {owner_address}
--validator-address {validator_address}
--fullnode-address {fullnode_address}
--chain-id {chain_id}
--local backend={backend};\
path={path};\
namespace={local_ns}
Expand All @@ -234,6 +239,7 @@ impl StorageHelper {
owner_address = owner_address,
validator_address = validator_address,
fullnode_address = fullnode_address,
chain_id = chain_id.id(),
backend = crate::secure_backend::DISK,
path = self.path_string(),
local_ns = local_ns,
Expand Down
4 changes: 4 additions & 0 deletions config/management/src/validator_config.rs
Expand Up @@ -22,6 +22,7 @@ use libra_secure_storage::{CryptoStorage, KVStorage, Storage, Value};
use libra_secure_time::{RealTimeService, TimeService};
use libra_types::{
account_address::{self, AccountAddress},
chain_id::ChainId,
transaction::{RawTransaction, SignedTransaction, Transaction},
};
use std::{convert::TryFrom, time::Duration};
Expand All @@ -38,6 +39,8 @@ pub struct ValidatorConfig {
fullnode_address: NetworkAddress,
#[structopt(flatten)]
backends: SecureBackends,
#[structopt(long)]
chain_id: ChainId,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is quite right, as it's essentially setting a chainID on a single transaction used during the genesis process (this transaction is submitted by each validator operator). Instead, my feeling is that we'll probably want to have the association determine the ID and set it in genesis once, before bootstrapping the network. However, from reading the comments it seems like there's still a lot of on-going discussions about how this should actually work..

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(If this is something that the association is going to decide at genesis, we may want to setup a sync with @davidiw, to figure out the best place to do this for the management tool)

}

impl ValidatorConfig {
Expand Down Expand Up @@ -122,6 +125,7 @@ impl ValidatorConfig {
constants::GAS_UNIT_PRICE,
constants::GAS_CURRENCY_CODE.to_owned(),
Duration::from_secs(expiration_time),
self.chain_id,
);
let signature = local_storage
.sign(OPERATOR_KEY, &raw_transaction)
Expand Down
18 changes: 0 additions & 18 deletions config/src/chain_id.rs

This file was deleted.

4 changes: 4 additions & 0 deletions config/src/config/key_manager_config.rs
Expand Up @@ -2,6 +2,7 @@
// SPDX-License-Identifier: Apache-2.0

use crate::config::{Error, LoggerConfig, PersistableConfig, SecureBackend};
use libra_types::chain_id::{self, ChainId};
use serde::{Deserialize, Serialize};
use std::path::{Path, PathBuf};

Expand All @@ -21,6 +22,8 @@ pub struct KeyManagerConfig {
pub secure_backend: SecureBackend,
pub sleep_period_secs: u64,
pub txn_expiration_secs: u64,
#[serde(deserialize_with = "chain_id::deserialize_config_chain_id")]
pub chain_id: ChainId,
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

chain ID needs to be added to key manager config too @mgorven @sausagee

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}

impl Default for KeyManagerConfig {
Expand All @@ -32,6 +35,7 @@ impl Default for KeyManagerConfig {
secure_backend: SecureBackend::InMemoryStorage,
sleep_period_secs: DEFAULT_SLEEP_PERIOD_SECS,
txn_expiration_secs: DEFAULT_TXN_EXPIRATION_SECS,
chain_id: ChainId::test(),
}
}
}
Expand Down
10 changes: 7 additions & 3 deletions config/src/config/mod.rs
Expand Up @@ -44,9 +44,12 @@ pub use safety_rules_config::*;
mod upstream_config;
pub use upstream_config::*;
mod test_config;
use crate::{chain_id::ChainId, network_id::NetworkId};
use crate::network_id::NetworkId;
use libra_secure_storage::{KVStorage, Storage};
use libra_types::waypoint::Waypoint;
use libra_types::{
chain_id::{self, ChainId},
waypoint::Waypoint,
};
pub use test_config::*;

/// Config pulls in configuration information from the config file.
Expand Down Expand Up @@ -91,6 +94,7 @@ pub struct NodeConfig {
#[serde(default, deny_unknown_fields)]
pub struct BaseConfig {
data_dir: PathBuf,
#[serde(deserialize_with = "chain_id::deserialize_config_chain_id")]
pub chain_id: ChainId,
pub role: RoleType,
pub waypoint: WaypointConfig,
Expand All @@ -100,7 +104,7 @@ impl Default for BaseConfig {
fn default() -> BaseConfig {
BaseConfig {
data_dir: PathBuf::from("/opt/libra/data/commmon"),
chain_id: ChainId::default(),
chain_id: ChainId::test(),
role: RoleType::Validator,
waypoint: WaypointConfig::None,
}
Expand Down
2 changes: 1 addition & 1 deletion config/src/config/test_data/public_full_node.yaml
@@ -1,5 +1,5 @@
base:
chain_id: "some-test-id"
chain_id: "TESTING"
data_dir: "/opt/libra/data/common"
role: "full_node"
waypoint:
Expand Down
2 changes: 1 addition & 1 deletion config/src/config/test_data/validator.yaml
@@ -1,5 +1,5 @@
base:
chain_id: "some-test-id"
chain_id: "TESTING"
data_dir: "/opt/libra/data/common"
role: "validator"
waypoint:
Expand Down
2 changes: 1 addition & 1 deletion config/src/config/test_data/validator_full_node.yaml
@@ -1,5 +1,5 @@
base:
chain_id: "some-test-id"
chain_id: "TESTING"
data_dir: "/opt/libra/data/common"
role: "full_node"
waypoint:
Expand Down
1 change: 0 additions & 1 deletion config/src/lib.rs
Expand Up @@ -3,7 +3,6 @@

#![forbid(unsafe_code)]

pub mod chain_id;
pub mod config;
pub mod generator;
pub mod keys;
Expand Down
2 changes: 2 additions & 0 deletions execution/executor-benchmark/src/lib.rs
Expand Up @@ -16,6 +16,7 @@ use libra_types::{
coin1_tag, libra_root_address, testnet_dd_account_address, AccountResource, COIN1_NAME,
},
block_info::BlockInfo,
chain_id::ChainId,
ledger_info::{LedgerInfo, LedgerInfoWithSignatures},
transaction::{
authenticator::AuthenticationKey, RawTransaction, Script, SignedTransaction, Transaction,
Expand Down Expand Up @@ -374,6 +375,7 @@ fn create_transaction(
0, /* gas_unit_price */
COIN1_NAME.to_owned(), /* gas_currency_code */
expiration_time,
ChainId::test(),
);

let signature = private_key.sign(&raw_txn);
Expand Down
5 changes: 4 additions & 1 deletion execution/executor/src/mock_vm/mod.rs
Expand Up @@ -10,6 +10,7 @@ use libra_types::{
access_path::AccessPath,
account_address::AccountAddress,
account_config::{libra_root_address, validator_set_address, LBR_NAME},
chain_id::ChainId,
contract_event::ContractEvent,
event::EventKey,
on_chain_config::{
Expand Down Expand Up @@ -314,6 +315,7 @@ fn encode_transaction(sender: AccountAddress, program: Script) -> Transaction {
0,
LBR_NAME.to_owned(),
std::time::Duration::from_secs(0),
ChainId::test(),
);

let privkey = Ed25519PrivateKey::generate_for_testing();
Expand All @@ -326,7 +328,8 @@ fn encode_transaction(sender: AccountAddress, program: Script) -> Transaction {
}

pub fn encode_reconfiguration_transaction(sender: AccountAddress) -> Transaction {
let raw_transaction = RawTransaction::new_write_set(sender, 0, WriteSet::default());
let raw_transaction =
RawTransaction::new_write_set(sender, 0, WriteSet::default(), ChainId::test());

let privkey = Ed25519PrivateKey::generate_for_testing();
Transaction::UserTransaction(
Expand Down
8 changes: 8 additions & 0 deletions json-rpc/json-rpc-spec.md
Expand Up @@ -1058,6 +1058,14 @@ User submitted transaction.
<td>Sequence number of this transaction corresponding to sender's account
</td>
</tr>
<tr>
<td>chain_id
</td>
<td>u64
</td>
<td>Chain ID of the network this transaction is intended for
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<td>Chain ID of the network this transaction is intended for
<td>Chain ID of the Libra network this transaction is intended for

</td>
</tr>
<tr>
<td>max_gas_amount
</td>
Expand Down
2 changes: 2 additions & 0 deletions json-rpc/src/tests/unit_tests.rs
Expand Up @@ -417,9 +417,11 @@ fn test_get_transactions() {
TransactionDataView::UserTransaction {
sender,
script_hash,
chain_id,
..
} => {
assert_eq!(&t.sender().to_string(), sender);
assert_eq!(&t.chain_id().id(), chain_id);
// TODO: verify every field
if let TransactionPayload::Script(s) = t.payload() {
assert_eq!(script_hash, &HashValue::sha3_256_of(s.code()).to_hex());
Expand Down