Rust

Code Standards

• In general, we follow the Rust community's established standards.
• Code must be formatted with rustfmt, and the clippy linter must not produce any warnings. Where necessary, exceptions can be made using #[allow(clippy::…)] and #[rustfmt::skip] attributes, but should be justified with a comment.
• The fmt-rfc's Rust Style Guide and the Rust API Guidelines contain some additional notes on de facto standards that don't need to be strictly followed but provide some orientation. In general, most of the API guidelines can be applied to each module as if it were a standalone crate, to make the code more maintainable and to enforce separation of concerns. E.g. private code should also follow common naming conventions and be well-documented. Note that even items that are not exported from the crate can have rustdoc comments (with "///"), and HTML documentation can be generated from those using cargo doc --document-private-items.

Proposed Standards

Don't commit Cargo.lock

Generally lockfiles shouldn't be committed for library crates and should for binary crates (see the Cargo FAQ for rationale).

We have both types of crates, and since we've chosen to use a workspace, all crates in that workspace share the same lockfile.  If not for this, we could split out all binaries into their own crates and commit their lockfiles.

As we have to make a choice, it's more prudent to not commit the lockfile.

Keep APIs as concise as possible

We should restrict all APIs to the minimum scope required.  This is normal programming practice.  It shouldn't be abandoned in the pursuit of generating rustdocs or doctests for private items.

Thorough documentation can still be applied to private code, and if it's felt that this needs to be augmented with compiling code examples, this can often be achieved through tests or examples.  Links to good test examples can be added to the doc comments.

Use abbreviated hex for Display and full hex for Debug output of byte strings

Some types contain byte strings as members (e.g. Vec<u8> or [u8; 32]).  To keep log output legible, we should decide if there's any benefit to actually including these in Display or Debug impls.

If so, we should use HexFmt; for Debug we should use the full output via {}, and for Display we should use the abbreviated form via {:10}.

If not, we should replace the actual data in the Display and Debug with "[{} bytes]" where the arg is field's length.

Log messages and Display impls

Keep these terse and all lowercase as far as possible.  Try to avoid multiple sentences, and don't finish log lines with a period.  The emphasis is on ease of debugging and structured logging rather than user-facing information.

Error types

• ensure they implement std::Error where possible, i.e. where not in a no_std environment (can be done via thiserror, avoid using failure)
• restrict usage of anyhow to test code or application code - don't use in library code

Imports

• split imports into blocks in the following order with a blank line between each block:
  1. std
  2. third party crates
  3. other CasperLabs crates
  4. current crate
• use the merged style
• bring structs, enums and macros fully into scope, bring functions' modules into scope so the function needs one level of qualification when called.  For very common struct types, it makes more sense to not bring these fully into scope.  For example, use std::fmt::{self, Debug, Formatter}; allows us to write

impl Debug for A {
    fn fmt(&self, formatter: &mut Formatter<'_>) -> fmt::Result {...}
}

Here, std::fmt::Result is not fully brought into scope as it would easily be confused with std::result::Result or one of the common Result aliases.

Log levels

We have 5 levels of logging available, which are ERROR, WARN, INFO, DEBUG and TRACE. Please follow the following guidelines when choosing a log level:

• ERROR: Only use this level if it would be okay to wake up Joe, Ed and then you (probably in that order) in the middle of the night. This is reserved for situations in which an assumed invariant does not hold! These are 100% a bug. Any ERROR level message found in the wild should trigger an immediate Jira ticket. ERROR is basically equivalent to panic (and often followed by one). Examples:
  • The hash of a verified deploy changed suddenly.
  • A lock was poisoned, even it never should be.
• WARN: This level is for errors that might be bugs, but are more likely to be failures that should not happen during the happy path operation. Potentially results in a Jira ticket after inspection. Examples:
  • Connection dropped mid-stream.
  • A node that sent a block is not able to produce the deploy for it.
• INFO: Events that do not occur frequently and are fairly informative are at the INFO level. If they are potentially breaking or important, more frequent events are also okay to be at the info level. It should almost always also be of interest to a node operator.
• DEBUG: Information that is neither frequent or informative. Basically stuff you will only glance at occasionally while debugging a feature. Please consider lowering the log level of newly added messages once you're done developing a feature and try to think from a node operator's perspective.
• TRACE: Everything else, information that is of very low entropy and/or frequently occurring. There's also the option of not logging something :)

More examples:

• Data corruption on storage: WARN - it is not an ERROR, because it might be outside interference.
• Hash collision detected (if we checked for those): ERROR. These do not happen, so it must be a bug :)
• A node spams us with invalid data: WARN once for when we ban the node, then DEBUG each time a connection is blocked from said validator.
• Block finalized: INFO This does not happen frequently but is of high interest.
• Deploy received: DEBUG. Arguably this could be INFO as well, but it is of high frequency.
• HTTP Request coming in: DEBUG or TRACE, your choice.
• Event discarded (because a specific reactor decided does not handle it): TRACE if the event is expected. ERROR if the event was never supposed to pop up here in the first place.

The rationale for these guidelines is that we need to cut our logs down in size a bit and make it easy for errors to stand out. A good approach for fixing bugs is looking at all the ERRORs, then all the WARNings immediately. Only with those gone, we dive into the INFO and below levels. It also makes it feasible to have actual alerts for ERRORs and WARNings.We're cleaning up some of the existing logs for this, so not everything will match this immediately, but please be mindful of your fellow devs when setting log levels.

Smart Contract Development

This section explains the smart contract development best practices that we as a team established while developing contracts for testing or demonstration purposes.

Error handling

• Don't use panic!
  
  Reason:  Using WebAssembly has this unfortunate side effect that error handling is complicated as we have to basically handle it ourselves. Currently, we don't have any tools to handle the panic message and report it somehow to the user. Instead, you're encouraged to use runtime::revert.

• Use custom Error enum in your contract code and revert with Error::User.
  
  This unifies the error values, and uses reserved revert ranges for user codes.
  
  Boilerplate code example to use:
  
  

  #[repr(u16)]
  enum Error {
      Foo = 0,
  }
  
  impl Into<ApiError> for Error {
      fn into(self) -> ApiError {
          ApiError::User(self as u16)
      }
  }
  
  fn call() { runtime::revert(Error::Foo); }

Python

Code Standards

• All python code must confirm to the official Pep 8 - Style Guide For Python

Code Formatting

• all code will be auto formatted using black
• this is enforced / done automatically by a pre-commit git hook

Scala

Don't's

• Using null in Scala code is not idiomatic at all.

  • private var author:Author = null

• If you can't give a default value, you really should use

  • var author: Option[Author] = None

• Don't rebind names for different uses:

  • Use vals

• Avoid using 's to overload reserved names:

  • typ instead of 'type'

• Don't prefix getters with get

  • money.count not money.getCount

• Do not use relative imports from other packages :

     Avoid

          import com.twitter

          import concurrent

    In favor of the unambiguous

          import com.twitter.concurrent

• The return keyword exits from the enclosing method, not the enclosing function. It behaves like a restricted throw, so don't use return unless it's in an exceptional situation where an API mandates error codes.  In all other situations, prefer Option, Either, case classes, or, if necessary, exceptions.
  
  
• Do not use structural types in normal use.

private type FooParam = {
 val baz: List[String => String]
 def bar(a: Int, b: Int): String
}
def foo(a: FooParam) = ...

Do's

• Use short names for small scopes:
  • is,js and ks are all but expected in loops
• Use longer names for larger scopes:
  • External API's should have longer and explanatory names that confer meaning,  Future.collect not Future.all.
• Use common abbreviations:
  • Ex: ok, err
• Use active names for operations with side effects:
  • user.activate() not user.setActive()
• Use descriptive names for methods that return values:
  • file.isDir not file.dir
• Use private[this] = visibility to the particular instance. Sometimes it aids performance optimizations.

• In singleton class types , visibility can be constrained by declaring the returned type:  

def foo(): Foo with Bar = new Foo with Bar with Baz {
            ...
      }

Traits

• Traits that contain implementation are not directly usable from Java: extend an abstract class with the trait instead.

// Not directly usable from Java
        trait Animal {
           def eat(other: Animal)
           def eatMany(animals: Seq[Animal) = animals foreach(eat(_))
         }
// But this is:
abstract class JavaAnimal extends Animal

• Keep traits short and orthogonal: don’t lump separable functionality into a trait, think of the smallest related ideas that fit together.   

  For example, imagine you have an something that can do IO:

  trait IOer {
          def write(bytes: Array[Byte])
          def read(n: Int): Array[Byte]
   }

separate the two behaviors:

  trait Reader {
        def read(n: Int): Array[Byte]
    }
   trait Writer {
       def write(bytes: Array[Byte])
   }

and mix them together to form what was an IOer:

new Reader with Writer…

Names

Don't repeat names that are already encapsulated in package or object name.
Prefer:

object User{
	def user(id: Int): Option[User]
}

Instead of:

object User{
	def getUser(id: Int): Option[User]
}

Pattern Matching

• Use pattern matching directly in function definitions whenever applicable

        Use this :    

   list map {
                 case Some(x) => x
                 case None => default
            }

  Instead of :

  list map { item =>
                  item match {
                      case Some(x) => x
                      case None => default
                 } }

  
  

• Pattern matching works best when also combined with destructuring

  Use this:

  animal match {
  	case Dog(breed) => "dog (%s)".format(breed)
  	case other => other.species
  }

  Instead of:

   animal match {
          case dog: Dog => "dog (%s)".format(dog.breed)
          case _ => animal.species
       }

Don't use pattern matching for conditional execution when defaults make more sense.

Use:

val x = list.headOption getOrElse default

NOT:

 val x = list match {
             case head :: _ => head
             case Nil => default
         }

• Scala provides an exception facility, but do not use it for commonplace errors, Instead, encode such errors explicitly: using Option

Use: 

trait Repository[Key, Value] {
          def get(key: Key): Option[Value]
     }

Instead of:

trait Repository[Key, Value] {
          def get(key: Key): Value
      }

• Use the following style:    

/**
     * ServiceBuilder builds services
     * ...
*/

/** ServiceBuilder builds services
     * ...
*/

def make(): Service = new Service{}

trait Service
def make() = new Service {
  def getId = 123
}

import scala.collection.mutable
val set = mutable.Set()

  val stream = getClass.getResourceAsStream("someclassdata")
               assert(stream != null)

def fib(n: Int) = {
require(n > 0)
  ...
  }

  try {
        operation()
      } catch {
          case _ => ...
      }    // almost always wrong, as it would catch fatal errors that need to be propagated.

import scala.util.control.NonFatal
...
  try {
      operation()
     } catch {
          case NonFatal(exc) => ...
    }

import scala.util.{Try, Success, Failure}
...
  val result: Try[ResultType] = Try( operation() )
  result match {
    case Success(v) => ... //do something with the return value
    case Failure(ex) => ... //non-fatal error handling here
  }

if (cond) {
  result
} else {
  otherResult
}

for (x <- y if cond) yield {
  operation(x)
}

while (notDone()) {
  doTheThing()
}

 val seq = Seq(1, 2, 3)
 val set = Set(1, 2, 3)
 val map = Map(1 -> "one", 2 -> "two", 3 -> "three")

for (item <- container) {
    if (item != 2) return
   }

trait SocketFactory extends (SocketAddress => Socket

type SocketFactory = SocketAddress => Socket

val addrToInet: SocketAddress => Long
val inetToSocket: Long => Socket
val factory: SocketFactory = addrToInet andThen inetToSocket

class ConcurrentPool[K, V] {
   type Queue = ConcurrentLinkedQueue[V]
   type Map   = ConcurrentHashMap[K, Queue]
 ...
}