Skip to content
This repository has been archived by the owner on Oct 22, 2023. It is now read-only.

warnings as errors #583

Merged
merged 14 commits into from
Feb 15, 2019
19 changes: 19 additions & 0 deletions .buildkite/rust/common.sh
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,25 @@ export INTEL_SGX_SDK="/opt/sgxsdk"
export EKIDEN_UNSAFE_SKIP_AVR_VERIFY="1"
export RUST_BACKTRACE="1"

##################################
# Set up RUSTFLAGS for the build #
##################################
if [ -z ${RUSTLINT+x} ]; then
RUSTLINT=""
for opt in $(cat .buildkite/rust/lint.txt | grep -v '#'); do
RUSTLINT=$RUSTLINT" -D "$opt
done

export RUSTLINT
if [ -z ${RUSTFLAGS+x} ]; then
export RUSTFLAGS=$RUSTLINT
else
export RUSTFLAGS=$RUSTFLAGS" "$RUSTLINT
fi

echo "Using RUSTFLAGS="$RUSTFLAGS
fi

####################################################
# By default, .bashrc will quit if the shell
# is not interactive. It checks whether $PS1 is
Expand Down
32 changes: 32 additions & 0 deletions .buildkite/rust/lint.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
#
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity is this an exhaustive list? I assume not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I got the list from here https://github.com/rust-unofficial/patterns/blob/master/anti_patterns/deny-warnings.md. It's not exhaustive since rust still adds/deprecates lints with quite some frequency. Based on that post, it looks like this is a reasonable set. Let me know if there's any other lint that you think should be added

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is great to start. We can add/subtract as necessary in the future.

# list of lints that will be used to compile the code
# elements after a '#' will be ignored
#
bad-style
const-err
dead-code
improper-ctypes
legacy-directory-ownership
non-shorthand-field-patterns
no-mangle-generic-items
overflowing-literals
path-statements
patterns-in-fns-without-body
plugin-as-library
private-in-public
safe-extern-statics
unconditional-recursion
unions-with-drop-fields
# unused
unused-allocation
unused-comparisons
unused-parens
while-true
# missing-debug-implementations
# missing-docs
# trivial-casts
# trivial-numeric-casts
# unused-extern-crates
# unused-import-braces
# unused-qualifications
# unused-results
4 changes: 4 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,7 @@ tests/rpc-tests/

# Coverage report from Tarpaulin
cobertura.xml

# in case credentials are put in the repo for using
# inside the docker container
.git-credentials
2 changes: 2 additions & 0 deletions api/build.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
//! Build crate for the ekiden api.

extern crate ekiden_tools;

fn main() {
Expand Down
1 change: 0 additions & 1 deletion api/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
extern crate protobuf;
extern crate serde;

#[macro_use]
extern crate serde_derive;
Expand Down
2 changes: 2 additions & 0 deletions build.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
//! build crate for the ekiden runtime based enclave
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
//! build crate for the ekiden runtime based enclave
//! Build crate for the ekiden runtime based enclave.


extern crate ekiden_edl;
extern crate ekiden_tools;

Expand Down
1 change: 1 addition & 0 deletions common/src/confidential/confidential_ctx.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![deny(warnings)]
use super::key_manager::KeyManagerClient;
use ekiden_core::mrae::{
nonce::{Nonce, NONCE_SIZE},
Expand Down
6 changes: 6 additions & 0 deletions common/src/confidential/key_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,21 @@ impl KeyManagerClient {
}
}

#[cfg(not(test))]
#[derive(Debug)]
/// Wrapper around the Ekiden key manager client to provide a more convenient
/// Ethereum address based interface along with runtime-specific utility methods.
struct KeyManager;
Copy link
Contributor

Choose a reason for hiding this comment

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

What made you delete this? We still need it. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it was just not being used

Copy link
Contributor

Choose a reason for hiding this comment

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

This is definitely used. See KeyManagerClient. We do some conditional compilation for testing.

Copy link
Contributor

Choose a reason for hiding this comment

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

The e2e tests will fail if this is deleted.


impl KeyManager {
#[allow(dead_code)]
/// Returns the contract id for the given contract address. The contract_id
/// is used to fetch keys for a contract.
fn contract_id(contract: Address) -> ContractId {
ContractId::from(&keccak(contract.to_vec())[..])
}

#[allow(dead_code)]
/// Creates and returns the long term public key for the given contract.
/// If the key already exists, returns the existing key.
/// Returns the tuple (public_key, signature_{KeyManager}(public_key)).
Expand All @@ -72,6 +77,7 @@ impl KeyManager {
})
}

#[allow(dead_code)]
fn contract_key(address: Address) -> Result<ContractKey, String> {
let contract_id = Self::contract_id(address);
let mut km = EkidenKeyManager::instance().expect("Should always have a key manager client");
Expand Down
7 changes: 1 addition & 6 deletions common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,6 @@ extern crate keccak_hash;
#[macro_use]
extern crate lazy_static;

#[cfg(not(target_env = "sgx"))]
extern crate rand;

#[cfg(target_env = "sgx")]
extern crate sgx_rand;

pub mod confidential;

use std::{
Expand Down Expand Up @@ -318,6 +312,7 @@ where
}
}

#[derive(Debug)]
/// Blockchain state database.
pub struct BlockchainStateDb<T: Database + Send + Sync> {
db: Mutex<T>,
Expand Down
2 changes: 1 addition & 1 deletion gateway/bin/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@

//! web3 gateway for Oasis Ethereum runtime.

#![deny(warnings)]
extern crate ctrlc;
extern crate fdlimit;
extern crate log;
extern crate parking_lot;
#[macro_use]
extern crate clap;
extern crate rand;

#[macro_use]
extern crate client_utils;
Expand Down
11 changes: 5 additions & 6 deletions gateway/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,11 @@ use hash::keccak;
use parity_rpc::v1::types::Bytes as RpcBytes;
use runtime_ethereum;
use runtime_ethereum_common::{confidential::has_confidential_prefix, State as EthState};
use std::time::{SystemTime, UNIX_EPOCH};
use traits::confidential::PublicKeyResult;
use transaction::{Action, LocalizedTransaction, SignedTransaction};

use client_utils::{self, db::Snapshot};
use ekiden_common::{bytes::B512, environment::Environment};
use ekiden_common::environment::Environment;
use ekiden_core::{error::Error, futures::prelude::*};
use ekiden_db_trusted::Database;
use ekiden_keymanager_client::KeyManager as EkidenKeyManager;
Expand Down Expand Up @@ -86,9 +85,11 @@ pub trait ChainNotify: Send + Sync {
pub struct Client {
client: runtime_ethereum::Client,
engine: Arc<EthEngine>,
#[allow(dead_code)]
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 dead can we delete it?

snapshot_manager: Option<client_utils::db::Manager>,
eip86_transition: u64,
environment: Arc<Environment>,
#[allow(dead_code)]
storage_backend: Arc<StorageBackend>,
/// The most recent block for which we have sent notifications.
notified_block_number: Mutex<BlockNumber>,
Expand Down Expand Up @@ -817,9 +818,9 @@ impl Client {
.is_confidential(transaction)
.map_err(|_| CallError::StateCorrupt)?
{
self.confidential_estimate_gas(transaction, id)
self.confidential_estimate_gas(transaction)
} else {
self._estimate_gas(transaction, id, db, state)
self._estimate_gas(transaction, db, state)
}
}

Expand All @@ -828,7 +829,6 @@ impl Client {
fn _estimate_gas<T: 'static + Database + Send + Sync>(
&self,
transaction: &SignedTransaction,
id: BlockId,
db: StateDb<T>,
mut state: EthState,
) -> Result<U256, CallError> {
Expand All @@ -849,7 +849,6 @@ impl Client {
fn confidential_estimate_gas(
&self,
transaction: &SignedTransaction,
id: BlockId,
) -> Result<U256, CallError> {
info!("estimating gas for a confidential contract");

Expand Down
15 changes: 3 additions & 12 deletions gateway/src/impls/confidential.rs
Original file line number Diff line number Diff line change
@@ -1,34 +1,25 @@
use client::Client;
use ekiden_common::bytes::B512;
use ekiden_core::futures::FutureExt;
use ekiden_keymanager_common::confidential;
use ethereum_api::TransactionRequest;
use ethereum_types::Address;
use impls::eth::EthClient;
use jsonrpc_core::{
futures::{future, Future},
BoxFuture, Error, ErrorCode, Result,
};
use jsonrpc_core::{futures::Future, BoxFuture, Error, ErrorCode, Result};
use jsonrpc_macros::Trailing;
use parity_rpc::v1::{
helpers::errors,
metadata::Metadata,
traits::Eth,
types::{BlockNumber, Bytes, CallRequest, H256},
types::{BlockNumber, Bytes, CallRequest},
};
use std::sync::Arc;
use traits::confidential::{Confidential, PublicKeyResult};

pub struct ConfidentialClient {
client: Arc<Client>,
eth_client: EthClient,
}

impl ConfidentialClient {
pub fn new(client: Arc<Client>) -> Self {
ConfidentialClient {
client: client.clone(),
eth_client: EthClient::new(&client),
}
}
}
Expand All @@ -45,7 +36,7 @@ impl Confidential for ConfidentialClient {

fn call_enc(
&self,
meta: Self::Metadata,
_meta: Self::Metadata,
request: CallRequest,
tag: Trailing<BlockNumber>,
) -> BoxFuture<Bytes> {
Expand Down
7 changes: 3 additions & 4 deletions gateway/src/impls/oasis.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
use std::{str::FromStr, sync::Arc};
use std::sync::Arc;

use ekiden_common::bytes::H256;
use ekiden_core::futures::Future;

use ethereum_types::Address;
use jsonrpc_core::{futures::future, BoxFuture, Error, ErrorCode, Result};
use jsonrpc_core::{BoxFuture, Error, ErrorCode};
use jsonrpc_macros::Trailing;
use parity_rpc::v1::types::{BlockNumber, H160 as RpcH160, H256 as RpcH256};
use parity_rpc::v1::types::{BlockNumber, H160 as RpcH160};

use client::Client;
use impls::eth::EthClient;
Expand Down
4 changes: 4 additions & 0 deletions gateway/src/informant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,15 @@ impl RpcStats {
.unwrap_or(0)
}

// used for tests
#[allow(dead_code)]
/// Returns number of open sessions
pub fn sessions(&self) -> usize {
self.sessions.read().len()
}

// used for tests
#[allow(dead_code)]
/// Returns requests rate
pub fn requests_rate(&self, id: &H256) -> usize {
self.sessions
Expand Down
19 changes: 1 addition & 18 deletions gateway/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,51 +18,35 @@

#[macro_use]
extern crate clap;
extern crate env_logger;
#[macro_use]
extern crate futures;
#[macro_use]
extern crate lazy_static;
#[macro_use]
extern crate log;
extern crate parking_lot;
extern crate path;
extern crate rayon;
extern crate regex;
Copy link
Contributor

Choose a reason for hiding this comment

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

Look at all of these removed crates. Glorious. Are they removed from Cargo.toml as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't, but we could do that. I would prefer to do it in a separate PR if that's okay though :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Why another PR? Might as well do it here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

let me try then

extern crate rustc_hex;
extern crate serde;
extern crate serde_json;
#[macro_use]
extern crate serde_derive;
extern crate toml;

extern crate jsonrpc_core;
#[macro_use]
extern crate jsonrpc_macros;
extern crate jsonrpc_http_server;
extern crate jsonrpc_ipc_server;
extern crate jsonrpc_pubsub;
extern crate jsonrpc_ws_server;

extern crate common_types;
#[macro_use]
extern crate ethcore;
extern crate ethcore_bytes as bytes;
extern crate ethcore_transaction as transaction;
extern crate ethereum_types;
extern crate evm;
#[cfg(test)]
extern crate hex;
extern crate journaldb;
extern crate keccak_hash as hash;
extern crate kvdb;
extern crate parity_machine;
extern crate parity_reactor;
extern crate parity_rpc;
extern crate rlp;
extern crate rlp_compress;
extern crate util_error;
extern crate vm;

#[macro_use]
extern crate client_utils;
Expand All @@ -77,7 +61,6 @@ extern crate ekiden_keymanager_common;
extern crate ekiden_registry_client;
#[cfg(test)]
extern crate ekiden_roothash_client;
extern crate ekiden_rpc_client;
#[cfg(test)]
extern crate ekiden_scheduler_client;
extern crate ekiden_storage_base;
Expand Down Expand Up @@ -119,7 +102,7 @@ use ethereum_types::U256;

use ekiden_core::{environment::Environment, x509};
use ekiden_runtime_client::create_runtime_client;
use ekiden_storage_base::{BackendIdentityMapper, StorageBackend};
use ekiden_storage_base::BackendIdentityMapper;
use ethereum_api::with_api;

pub use self::run::RunningClient;
Expand Down
6 changes: 0 additions & 6 deletions gateway/src/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,6 @@ pub struct HttpConfiguration {
pub max_batch_size: usize,
}

impl HttpConfiguration {
pub fn address(&self) -> Option<rpc::Host> {
address(self.enabled, &self.interface, self.port, &self.hosts)
}
}

impl Default for HttpConfiguration {
fn default() -> Self {
HttpConfiguration {
Expand Down
2 changes: 2 additions & 0 deletions gateway/src/rpc_apis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ impl FromStr for Api {

#[derive(Debug, Clone)]
pub enum ApiSet {
// Used in tests.
#[allow(dead_code)]
// Safe context (like token-protected WS interface)
SafeContext,
// Unsafe context (like jsonrpc over http)
Expand Down
3 changes: 1 addition & 2 deletions gateway/src/traits/confidential.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
use ekiden_common::bytes::B512;
use ethereum_types::Address;
use jsonrpc_core::{BoxFuture, Result};
use jsonrpc_macros::Trailing;
use parity_rpc::v1::types::{BlockNumber, Bytes, CallRequest, H256};
use parity_rpc::v1::types::{BlockNumber, Bytes, CallRequest};

build_rpc_trait! {
pub trait Confidential {
Expand Down
Loading