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

Add coding guidelines #917

Merged
merged 8 commits into from
Aug 9, 2018
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
52 changes: 52 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
Solana Coding Guidelines
===

The goal of these guidelines is to improve developer productivity by allowing developers to
jump any file in the codebase and not need to adapt to inconsistencies in how the code is
written. The codebase should appear as if it had been authored by a single developer. If you
don't agree with a convention, submit a PR patching this document and let's discuss! Once
the PR is accepted, *all* code should be updated as soon as possible to reflect the new
conventions.

Rust coding conventions
---

* All Rust code is formatted using the latest version of `rustfmt`. Once installed, it will be
updated automatically when you update the compiler with `rustup`.

* All Rust code is linted with Clippy. If you'd prefer to ignore its advice, do so explicitly:

```rust
#[cfg_attr(feature = "cargo-clippy", allow(too_many_arguments))]
```

Note: Clippy defaults can be overridden in the top-level file `.clippy.toml`.

* For variable names, when in doubt, spell it out. The mapping from type names to variable names
is to lowercase the type name, putting an underscore before each capital letter. Variable names
should *not* be abbreviated unless being used as closure arguments and the brevity improves
readability. When a function has multiple instances of the same type, qualify each with a
prefix and underscore (i.e. alice_keypair) or a numeric suffix (i.e. tx0).

* For function and method names, use `<verb>_<subject>`. For unit tests, that verb should
always be `test` and for benchmarks the verb should always be `bench`. Avoid namespacing
function names with some arbitrary word. Avoid abreviating words in function names.

* As they say, "When in Rome, do as the Romans do." A good patch should acknowledge the coding
conventions of the code that surrounds it, even in the case where that code has not yet been
updated to meet the conventions described here.


Terminology
---

Inventing new terms is allowed, but should only be done when the term is widely used and
understood. Avoid introducing new 3-letter terms, which can be confused with 3-letter acronyms.

Some terms we currently use regularly in the codebase:

* hash: n. A SHA-256 Hash
* keypair: n. A Ed25519 key-pair, containing a public and private key.
* pubkey: n. The public key of a Ed25519 key-pair.
* sigverify: v. To verify a Ed25519 digital signature.

6 changes: 3 additions & 3 deletions benches/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use rayon::prelude::*;
use solana::bank::*;
use solana::hash::hash;
use solana::mint::Mint;
use solana::signature::{KeyPair, KeyPairUtil};
use solana::signature::{Keypair, KeypairUtil};
use solana::transaction::Transaction;

fn bench_process_transaction(bencher: &mut Bencher) {
Expand All @@ -22,15 +22,15 @@ fn bench_process_transaction(bencher: &mut Bencher) {
.into_par_iter()
.map(|i| {
// Seed the 'from' account.
let rando0 = KeyPair::new();
let rando0 = Keypair::new();
let tx = Transaction::new(&mint.keypair(), rando0.pubkey(), 10_000, mint.last_id());
assert!(bank.process_transaction(&tx).is_ok());

// Seed the 'to' account and a cell for its signature.
let last_id = hash(&serialize(&i).unwrap()); // Unique hash
bank.register_entry_id(&last_id);

let rando1 = KeyPair::new();
let rando1 = Keypair::new();
let tx = Transaction::new(&rando0, rando1.pubkey(), 1, last_id);
assert!(bank.process_transaction(&tx).is_ok());

Expand Down
14 changes: 7 additions & 7 deletions benches/banking_stage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use solana::banking_stage::BankingStage;
use solana::mint::Mint;
use solana::packet::{to_packets_chunked, PacketRecycler};
use solana::record_stage::Signal;
use solana::signature::{KeyPair, KeyPairUtil};
use solana::signature::{Keypair, KeypairUtil};
use solana::transaction::Transaction;
use std::iter;
use std::sync::mpsc::{channel, Receiver};
Expand All @@ -23,7 +23,7 @@ use std::sync::Arc;
// use hash::hash;
// use mint::Mint;
// use rayon::prelude::*;
// use signature::{KeyPair, KeyPairUtil};
// use signature::{Keypair, KeypairUtil};
// use std::collections::HashSet;
// use std::time::Instant;
// use transaction::Transaction;
Expand All @@ -49,11 +49,11 @@ use std::sync::Arc;
// }
//
// // Seed the 'from' account.
// let rando0 = KeyPair::new();
// let rando0 = Keypair::new();
// let tx = Transaction::new(&mint.keypair(), rando0.pubkey(), 1_000, last_id);
// bank.process_transaction(&tx).unwrap();
//
// let rando1 = KeyPair::new();
// let rando1 = Keypair::new();
// let tx = Transaction::new(&rando0, rando1.pubkey(), 2, last_id);
// bank.process_transaction(&tx).unwrap();
//
Expand Down Expand Up @@ -102,9 +102,9 @@ fn bench_banking_stage_multi_accounts(bencher: &mut Bencher) {
let num_dst_accounts = 8 * 1024;
let num_src_accounts = 8 * 1024;

let srckeys: Vec<_> = (0..num_src_accounts).map(|_| KeyPair::new()).collect();
let srckeys: Vec<_> = (0..num_src_accounts).map(|_| Keypair::new()).collect();
let dstkeys: Vec<_> = (0..num_dst_accounts)
.map(|_| KeyPair::new().pubkey())
.map(|_| Keypair::new().pubkey())
.collect();

let transactions: Vec<_> = (0..tx)
Expand Down Expand Up @@ -175,7 +175,7 @@ fn bench_banking_stage_single_from(bencher: &mut Bencher) {
let mut pubkeys = Vec::new();
let num_keys = 8;
for _ in 0..num_keys {
pubkeys.push(KeyPair::new().pubkey());
pubkeys.push(Keypair::new().pubkey());
}

let transactions: Vec<_> = (0..tx)
Expand Down
4 changes: 2 additions & 2 deletions benches/ledger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@ use criterion::{Bencher, Criterion};
use solana::hash::{hash, Hash};
use solana::ledger::{next_entries, reconstruct_entries_from_blobs, Block};
use solana::packet::BlobRecycler;
use solana::signature::{KeyPair, KeyPairUtil};
use solana::signature::{Keypair, KeypairUtil};
use solana::transaction::Transaction;
use std::collections::VecDeque;

fn bench_block_to_blobs_to_block(bencher: &mut Bencher) {
let zero = Hash::default();
let one = hash(&zero.as_ref());
let keypair = KeyPair::new();
let keypair = Keypair::new();
let tx0 = Transaction::new(&keypair, keypair.pubkey(), 1, one);
let transactions = vec![tx0; 10];
let entries = next_entries(&zero, 1, transactions);
Expand Down
6 changes: 3 additions & 3 deletions benches/sigverify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use solana::packet::{to_packets, PacketRecycler};
use solana::sigverify;
use solana::transaction::test_tx;

fn bench_sig_verify(bencher: &mut Bencher) {
fn bench_sigverify(bencher: &mut Bencher) {
let tx = test_tx();

// generate packet vector
Expand All @@ -23,8 +23,8 @@ fn bench_sig_verify(bencher: &mut Bencher) {
}

fn bench(criterion: &mut Criterion) {
criterion.bench_function("bench_sig_verify", |bencher| {
bench_sig_verify(bencher);
criterion.bench_function("bench_sigverify", |bencher| {
bench_sigverify(bencher);
});
}

Expand Down
Loading