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

Switch to Github Actions #399

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all 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
84 changes: 0 additions & 84 deletions .circleci/config.yml

This file was deleted.

27 changes: 27 additions & 0 deletions .github/workflows/audit.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
on:
pull_request:
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't cargo audit be run on: push, instead of on: pull_request?

paths-ignore:
- "docs/**/*"
- "scripts/**/*"
- "README.md"

name: Audit Dependencies

jobs:
audit:
runs-on: ubuntu-latest
Copy link
Contributor

Choose a reason for hiding this comment

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

What are the options here? Is this the platform that we encourage people to use?

Copy link
Member Author

Choose a reason for hiding this comment

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

Virtual environment YAML workflow label
Windows Server 2019 windows-latest or windows-2019
Windows Server 2016 R2 windows-2016
Ubuntu 18.04 ubuntu-latest or ubuntu-18.04
Ubuntu 16.04 ubuntu-16.04
macOS X Mojave 10.14 macOS-latest or macOS-10.14

Source

Not necessarily, but it's what we were building on Circle already. If we want to support others, we can add steps to build it on other platforms too (as a separate PR)

steps:
- name: Checkout sources
uses: actions/checkout@v1

- name: Install stable toolchain
uses: actions-rs/toolchain@v1
with:
toolchain: stable
override: true

- name: Install cargo-audit
run: cargo install cargo-audit

- name: Audit dependencies
run: cargo audit
22 changes: 22 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
name: Build and Test
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a semantic reason why the name is at the top for this one?


on:
push:
paths-ignore:
- "docs/**/*"
- "scripts/**/*"
- "README.md"

jobs:
build:
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v1
- name: Build
run: cargo build --all-targets --all-features
- name: Install redis
run: sudo apt install redis-server && redis-server --version
- name: Run tests
run: cargo test --all --all-features
timeout-minutes: 10
48 changes: 48 additions & 0 deletions .github/workflows/style.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
on:
push:
paths-ignore:
- "docs/**/*"
- "scripts/**/*"
- "README.md"

name: Check Style

jobs:
rustfmt:
runs-on: ubuntu-latest
steps:
- name: Checkout sources
uses: actions/checkout@v1

- name: Install stable toolchain
uses: actions-rs/toolchain@v1
with:
toolchain: stable
override: true

- name: Install rustfmt
run: rustup component add rustfmt

- name: Run cargo fmt
uses: actions-rs/cargo@v1
with:
command: fmt
args: --all -- --check

clippy:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v1
- id: component
uses: actions-rs/components-nightly@v1
Copy link
Member

@gakonst gakonst Oct 14, 2019

Choose a reason for hiding this comment

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

Should we be using nightly here? Just being conservative in case nightly clippy tells us to do something that works in nightly, but does not work on stable

with:
component: clippy
- uses: actions-rs/toolchain@v1
with:
toolchain: ${{ steps.component.outputs.toolchain }}
override: true
- run: rustup component add clippy
- uses: actions-rs/clippy-check@v1
with:
token: ${{ secrets.GITHUB_TOKEN }}
args: --all-targets --all-features -- -D warnings
5 changes: 2 additions & 3 deletions crates/interledger-ccp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@
use bytes::Bytes;
use futures::Future;
use interledger_service::Account;
use std::collections::HashMap;
use std::{str::FromStr, string::ToString};
use std::{collections::HashMap, str::FromStr, string::ToString};

#[cfg(test)]
mod fixtures;
Expand All @@ -30,7 +29,7 @@ use serde::{Deserialize, Serialize};

/// Data structure used to describe the routing relation of an account with its peers.
#[repr(u8)]
#[derive(Clone, Copy, Debug, PartialEq, PartialOrd, Serialize, Deserialize)]
#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)]
pub enum RoutingRelation {
/// An account from which we do not receive routes from, neither broadcast
/// routes to
Expand Down
42 changes: 20 additions & 22 deletions crates/interledger-ccp/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@ use parking_lot::{Mutex, RwLock};
use ring::digest::{digest, SHA256};
use std::collections::HashMap;
use std::{
cmp::min,
cmp::{min, Ordering},
convert::TryFrom,
str,
sync::{
atomic::{AtomicU32, Ordering},
atomic::{self, AtomicU32},
Arc,
},
time::{Duration, Instant},
Expand Down Expand Up @@ -687,7 +687,7 @@ where
.and_then(move |mut accounts| {
let mut outgoing = self_clone.outgoing.clone();
let to_epoch_index = self_clone.forwarding_table.read().epoch();
let from_epoch_index = self_clone.last_epoch_updates_sent_for.swap(to_epoch_index, Ordering::SeqCst);
let from_epoch_index = self_clone.last_epoch_updates_sent_for.swap(to_epoch_index, atomic::Ordering::SeqCst);

let route_update_request =
self_clone.create_route_update(from_epoch_index, to_epoch_index);
Expand Down Expand Up @@ -933,25 +933,23 @@ fn get_best_route_for_prefix<A: CcpRoutingAccount>(
let (best_account, best_route) = candidate_routes.fold(
(account, route),
|(best_account, best_route), (account, route)| {
// Prioritize child > peer > parent
if best_account.routing_relation() > account.routing_relation() {
return (best_account, best_route);
} else if best_account.routing_relation() < account.routing_relation() {
return (account, route);
}

// Prioritize shortest path
if best_route.path.len() < route.path.len() {
return (best_account, best_route);
} else if best_route.path.len() > route.path.len() {
return (account, route);
}

// Finally base it on account ID
if best_account.id().to_string() < account.id().to_string() {
(best_account, best_route)
} else {
(account, route)
// Priority:
// 1. child > peer > parent
// 2. shortest path
// 3. account ID (random priority)
match (
best_account
.routing_relation()
.cmp(&account.routing_relation()),
best_route.path.len().cmp(&route.path.len()),
best_account.id().to_string().cmp(&account.id().to_string()),
) {
(Ordering::Greater, _, _)
| (Ordering::Equal, Ordering::Less, _)
| (Ordering::Equal, Ordering::Equal, Ordering::Less) => {
(best_account, best_route)
}
_ => (account, route),
emschwartz marked this conversation as resolved.
Show resolved Hide resolved
}
},
);
Expand Down
69 changes: 36 additions & 33 deletions crates/interledger-settlement/src/test_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@ use crate::fixtures::{BODY, MESSAGES_API, SERVICE_ADDRESS, SETTLEMENT_API, TEST_
use lazy_static::lazy_static;
use parking_lot::RwLock;
use std::collections::HashMap;
use std::str::FromStr;
use std::sync::Arc;
use std::{cmp::Ordering, str::FromStr, sync::Arc};
use tokio::runtime::Runtime;
use url::Url;

Expand Down Expand Up @@ -184,37 +183,41 @@ impl LeftoversStore for TestStore {
) -> Box<dyn Future<Item = (), Error = ()> + Send> {
let mut guard = self.uncredited_settlement_amount.write();
if let Some(leftovers) = (*guard).get_mut(&account_id) {
if leftovers.1 > uncredited_settlement_amount.1 {
// the current leftovers maintain the scale so we just need to
// upscale the provided leftovers to the existing leftovers' scale
let scaled = uncredited_settlement_amount
.0
.normalize_scale(ConvertDetails {
from: uncredited_settlement_amount.1,
to: leftovers.1,
})
.unwrap();
*leftovers = (leftovers.0.clone() + scaled, leftovers.1);
} else if leftovers.1 == uncredited_settlement_amount.1 {
*leftovers = (
leftovers.0.clone() + uncredited_settlement_amount.0,
leftovers.1,
);
} else {
// if the scale of the provided leftovers is bigger than
// existing scale then we update the scale of the leftovers'
// scale
let scaled = leftovers
.0
.normalize_scale(ConvertDetails {
from: leftovers.1,
to: uncredited_settlement_amount.1,
})
.unwrap();
*leftovers = (
uncredited_settlement_amount.0 + scaled,
uncredited_settlement_amount.1,
);
match leftovers.1.cmp(&uncredited_settlement_amount.1) {
Ordering::Greater => {
// the current leftovers maintain the scale so we just need to
// upscale the provided leftovers to the existing leftovers' scale
let scaled = uncredited_settlement_amount
.0
.normalize_scale(ConvertDetails {
from: uncredited_settlement_amount.1,
to: leftovers.1,
})
.unwrap();
*leftovers = (leftovers.0.clone() + scaled, leftovers.1);
}
Ordering::Equal => {
*leftovers = (
leftovers.0.clone() + uncredited_settlement_amount.0,
leftovers.1,
);
}
Ordering::Less => {
// if the scale of the provided leftovers is bigger than
// existing scale then we update the scale of the leftovers'
// scale
let scaled = leftovers
.0
.normalize_scale(ConvertDetails {
from: leftovers.1,
to: uncredited_settlement_amount.1,
})
.unwrap();
*leftovers = (
uncredited_settlement_amount.0 + scaled,
uncredited_settlement_amount.1,
);
}
}
} else {
(*guard).insert(account_id, uncredited_settlement_amount);
Expand Down