Skip to content

Wrap Comments #1003

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

Closed
wants to merge 3 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
26 changes: 13 additions & 13 deletions .rustfmt.toml
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
max_width = 90 # changed
max_width = 90
hard_tabs = false
tab_spaces = 4
newline_style = "Auto"
use_small_heuristics = "Default"
indent_style = "Block"
wrap_comments = false
wrap_comments = true
format_code_in_doc_comments = false
comment_width = 80
normalize_comments = true # changed
normalize_comments = true
normalize_doc_attributes = false
license_template_path = "FILE_HEADER" # changed
license_template_path = "FILE_HEADER"
format_strings = false
format_macro_matchers = false
format_macro_bodies = true
Expand All @@ -18,8 +18,8 @@ struct_lit_single_line = true
fn_single_line = false
where_single_line = false
imports_indent = "Block"
imports_layout = "Vertical" # changed
imports_granularity = "Crate" # changed
imports_layout = "Vertical"
imports_granularity = "Crate"
reorder_imports = true
reorder_modules = true
reorder_impl_items = false
Expand All @@ -29,29 +29,29 @@ space_after_colon = true
spaces_around_ranges = false
binop_separator = "Front"
remove_nested_parens = true
combine_control_expr = false # changed
combine_control_expr = false
overflow_delimited_expr = false
struct_field_align_threshold = 0
enum_discrim_align_threshold = 0
match_arm_blocks = true
force_multiline_blocks = true # changed
force_multiline_blocks = true
fn_args_layout = "Tall"
brace_style = "SameLineWhere"
control_brace_style = "AlwaysSameLine"
trailing_semicolon = false # changed
trailing_semicolon = false
trailing_comma = "Vertical"
match_block_trailing_comma = false
blank_lines_upper_bound = 1
blank_lines_lower_bound = 0
edition = "2021" # changed
edition = "2021"
version = "One"
merge_derives = true
use_try_shorthand = true # changed
use_field_init_shorthand = true # changed
use_try_shorthand = true
use_field_init_shorthand = true
force_explicit_abi = true
condense_wildcard_suffixes = false
color = "Auto"
unstable_features = true # changed
unstable_features = true
disable_all_formatting = false
skip_children = false
hide_parse_errors = false
Expand Down
93 changes: 51 additions & 42 deletions crates/allocator/src/bump.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,14 @@

//! A simple bump allocator.
//!
//! Its goal to have a much smaller footprint than the admittedly more full-featured `wee_alloc`
//! allocator which is currently being used by ink! smart contracts.
//! Its goal to have a much smaller footprint than the admittedly more
//! full-featured `wee_alloc` allocator which is currently being used by ink!
//! smart contracts.
//!
//! The heap which is used by this allocator is built from pages of Wasm memory (each page is `64KiB`).
//! We will request new pages of memory as needed until we run out of memory, at which point we
//! will crash with an `OOM` error instead of freeing any memory.
//! The heap which is used by this allocator is built from pages of Wasm memory
//! (each page is `64KiB`). We will request new pages of memory as needed until
//! we run out of memory, at which point we will crash with an `OOM` error
//! instead of freeing any memory.

use core::alloc::{
GlobalAlloc,
Expand All @@ -45,8 +47,8 @@ unsafe impl GlobalAlloc for BumpAllocator {

#[inline]
unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 {
// A new page in Wasm is guaranteed to already be zero initialized, so we can just use our
// regular `alloc` call here and save a bit of work.
// A new page in Wasm is guaranteed to already be zero initialized, so we can
// just use our regular `alloc` call here and save a bit of work.
//
// See: https://webassembly.github.io/spec/core/exec/modules.html#growing-memories
self.alloc(layout)
Expand Down Expand Up @@ -110,10 +112,12 @@ impl InnerAlloc {
}
}

/// Tries to allocate enough memory on the heap for the given `Layout`. If there is not enough
/// room on the heap it'll try and grow it by a page.
/// Tries to allocate enough memory on the heap for the given `Layout`. If
/// there is not enough room on the heap it'll try and grow it by a
/// page.
///
/// Note: This implementation results in internal fragmentation when allocating across pages.
/// Note: This implementation results in internal fragmentation when
/// allocating across pages.
fn alloc(&mut self, layout: Layout) -> Option<usize> {
let alloc_start = self.next;

Expand All @@ -137,11 +141,12 @@ impl InnerAlloc {
}
}

/// Calculates the number of pages of memory needed for an allocation of `size` bytes.
/// Calculates the number of pages of memory needed for an allocation of `size`
/// bytes.
///
/// This function rounds up to the next page. For example, if we have an allocation of
/// `size = PAGE_SIZE / 2` this function will indicate that one page is required to satisfy
/// the allocation.
/// This function rounds up to the next page. For example, if we have an
/// allocation of `size = PAGE_SIZE / 2` this function will indicate that one
/// page is required to satisfy the allocation.
#[inline]
fn required_pages(size: usize) -> Option<usize> {
size.checked_add(PAGE_SIZE - 1)
Expand Down Expand Up @@ -235,8 +240,8 @@ mod tests {
let expected_limit = 2 * PAGE_SIZE;
assert_eq!(inner.upper_limit, expected_limit);

// Notice that we start the allocation on the second page, instead of making use of the
// remaining byte on the first page
// Notice that we start the allocation on the second page, instead of making use
// of the remaining byte on the first page
let expected_alloc_start = PAGE_SIZE + size_of::<u16>();
assert_eq!(inner.next, expected_alloc_start);
}
Expand All @@ -259,8 +264,8 @@ mod tests {
let expected_alloc_start = size_of::<Foo>();
assert_eq!(inner.next, expected_alloc_start);

// Now we want to make sure that the state of our allocator is correct for any subsequent
// allocations
// Now we want to make sure that the state of our allocator is correct for any
// subsequent allocations
let layout = Layout::new::<u8>();
assert_eq!(inner.alloc(layout), Some(2 * PAGE_SIZE));

Expand Down Expand Up @@ -288,8 +293,8 @@ mod fuzz_tests {

#[quickcheck]
fn should_allocate_arbitrary_sized_bytes(n: usize) -> TestResult {
// If `n` is going to overflow we don't want to check it here (we'll check the overflow
// case in another test)
// If `n` is going to overflow we don't want to check it here (we'll check the
// overflow case in another test)
if n.checked_add(PAGE_SIZE - 1).is_none() {
return TestResult::discard()
}
Expand Down Expand Up @@ -323,7 +328,8 @@ mod fuzz_tests {

#[quickcheck]
fn should_not_allocate_arbitrary_bytes_if_they_overflow(n: usize) -> TestResult {
// In the previous test we ignored the overflow case, now we ignore the valid cases
// In the previous test we ignored the overflow case, now we ignore the valid
// cases
if n.checked_add(PAGE_SIZE - 1).is_some() {
return TestResult::discard()
}
Expand Down Expand Up @@ -351,8 +357,8 @@ mod fuzz_tests {
let aligns = [1, 2, 4, 8, 16, 32, 64, 128, 256, 512];
let align = aligns[align % aligns.len()];

// If `n` is going to overflow we don't want to check it here (we'll check the overflow
// case in another test)
// If `n` is going to overflow we don't want to check it here (we'll check the
// overflow case in another test)
if n.checked_add(PAGE_SIZE - 1).is_none() {
return TestResult::discard()
}
Expand Down Expand Up @@ -382,15 +388,16 @@ mod fuzz_tests {
TestResult::passed()
}

/// The idea behind this fuzz test is to check a series of allocation sequences. For
/// example, we maybe have back to back runs as follows:
/// The idea behind this fuzz test is to check a series of allocation
/// sequences. For example, we maybe have back to back runs as follows:
///
/// 1. `vec![1, 2, 3]`
/// 2. `vec![4, 5, 6, 7]`
/// 3. `vec![8]`
///
/// Each of the vectors represents one sequence of allocations. Within each sequence the
/// individual size of allocations will be randomly selected by `quickcheck`.
/// Each of the vectors represents one sequence of allocations. Within each
/// sequence the individual size of allocations will be randomly
/// selected by `quickcheck`.
#[quickcheck]
fn should_allocate_arbitrary_byte_sequences(sequence: Vec<usize>) -> TestResult {
let mut inner = InnerAlloc::new();
Expand All @@ -399,8 +406,8 @@ mod fuzz_tests {
return TestResult::discard()
}

// We want to make sure no single allocation is going to overflow, we'll check this
// case in a different test
// We want to make sure no single allocation is going to overflow, we'll check
// this case in a different test
if !sequence
.iter()
.all(|n| n.checked_add(PAGE_SIZE - 1).is_some())
Expand All @@ -415,8 +422,8 @@ mod fuzz_tests {
.fold(0, |acc, &x| acc + required_pages(x).unwrap());
let max_pages = required_pages(usize::MAX - PAGE_SIZE + 1).unwrap();

// We know this is going to end up overflowing, we'll check this case in a different
// test
// We know this is going to end up overflowing, we'll check this case in a
// different test
if pages_required > max_pages {
return TestResult::discard()
}
Expand All @@ -442,8 +449,8 @@ mod fuzz_tests {

total_bytes_fragmented += fragmented_in_current_page;

// We expect our next allocation to be aligned to the start of the next page
// boundary
// We expect our next allocation to be aligned to the start of the next
// page boundary
expected_alloc_start = inner.upper_limit;
}

Expand Down Expand Up @@ -471,11 +478,12 @@ mod fuzz_tests {
TestResult::passed()
}

// For this test we have sequences of allocations which will eventually overflow the maximum
// amount of pages (in practice this means our heap will be OOM).
// For this test we have sequences of allocations which will eventually overflow
// the maximum amount of pages (in practice this means our heap will be
// OOM).
//
// We don't care about the allocations that succeed (those are checked in other tests), we just
// care that eventually an allocation doesn't success.
// We don't care about the allocations that succeed (those are checked in other
// tests), we just care that eventually an allocation doesn't success.
#[quickcheck]
fn should_not_allocate_arbitrary_byte_sequences_which_eventually_overflow(
sequence: Vec<usize>,
Expand All @@ -486,8 +494,8 @@ mod fuzz_tests {
return TestResult::discard()
}

// We want to make sure no single allocation is going to overflow, we'll check that
// case seperately
// We want to make sure no single allocation is going to overflow, we'll check
// that case seperately
if !sequence
.iter()
.all(|n| n.checked_add(PAGE_SIZE - 1).is_some())
Expand All @@ -502,8 +510,8 @@ mod fuzz_tests {
.fold(0, |acc, &x| acc + required_pages(x).unwrap());
let max_pages = required_pages(usize::MAX - PAGE_SIZE + 1).unwrap();

// We want to explicitly test for the case where a series of allocations eventually
// runs out of pages of memory
// We want to explicitly test for the case where a series of allocations
// eventually runs out of pages of memory
if !(pages_required > max_pages) {
return TestResult::discard()
}
Expand All @@ -515,7 +523,8 @@ mod fuzz_tests {
results.push(inner.alloc(layout));
}

// Ensure that at least one of the allocations ends up overflowing our calculations.
// Ensure that at least one of the allocations ends up overflowing our
// calculations.
assert!(
results.iter().any(|r| r.is_none()),
"Expected an allocation to overflow our heap, but this didn't happen."
Expand Down
15 changes: 9 additions & 6 deletions crates/allocator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,20 @@
// See the License for the specific language governing permissions and
// limitations under the License.

//! Crate providing allocator support for all Wasm compilations of ink! smart contracts.
//! Crate providing allocator support for all Wasm compilations of ink! smart
//! contracts.
//!
//! The default allocator is a bump allocator whose goal is to have a small size footprint. If you
//! are not concerned about the size of your final Wasm binaries you may opt into using the more
//! full-featured `wee_alloc` allocator by activating the `wee-alloc` crate feature.
//! The default allocator is a bump allocator whose goal is to have a small size
//! footprint. If you are not concerned about the size of your final Wasm
//! binaries you may opt into using the more full-featured `wee_alloc` allocator
//! by activating the `wee-alloc` crate feature.

#![cfg_attr(not(feature = "std"), no_std)]
#![cfg_attr(not(feature = "std"), feature(alloc_error_handler, core_intrinsics))]

// We use `wee_alloc` as the global allocator since it is optimized for binary file size
// so that contracts compiled with it as allocator do not grow too much in size.
// We use `wee_alloc` as the global allocator since it is optimized for binary
// file size so that contracts compiled with it as allocator do not grow too
// much in size.
#[cfg(not(feature = "std"))]
#[cfg(feature = "wee-alloc")]
#[global_allocator]
Expand Down
10 changes: 6 additions & 4 deletions crates/engine/src/database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@ impl Database {
self.hmap.get(&hashed_key.to_vec())
}

/// Inserts `value` into the contract storage of `account_id` at storage key `key`.
/// Inserts `value` into the contract storage of `account_id` at storage key
/// `key`.
pub fn insert_into_contract_storage(
&mut self,
account_id: &[u8],
Expand All @@ -84,7 +85,8 @@ impl Database {
self.hmap.insert(hashed_key.to_vec(), value)
}

/// Removes the value at the contract storage of `account_id` at storage key `key`.
/// Removes the value at the contract storage of `account_id` at storage key
/// `key`.
pub fn remove_contract_storage(
&mut self,
account_id: &[u8],
Expand All @@ -94,8 +96,8 @@ impl Database {
self.hmap.remove(&hashed_key.to_vec())
}

/// Removes a key from the storage, returning the value at the key if the key
/// was previously in storage.
/// Removes a key from the storage, returning the value at the key if the
/// key was previously in storage.
pub fn remove(&mut self, key: &[u8]) -> Option<Vec<u8>> {
self.hmap.remove(key)
}
Expand Down
16 changes: 8 additions & 8 deletions crates/engine/src/exec_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,17 @@ use super::types::{
pub struct ExecContext {
/// The caller of the contract execution. Might be user or another contract.
///
/// We don't know the specifics of the AccountId ‒ like how many bytes or what
/// type of default `AccountId` makes sense ‒ they are left to be initialized
/// by the crate which uses the `engine`. Methods which require a caller might
/// panic when it has not been set.
/// We don't know the specifics of the AccountId ‒ like how many bytes or
/// what type of default `AccountId` makes sense ‒ they are left to be
/// initialized by the crate which uses the `engine`. Methods which
/// require a caller might panic when it has not been set.
pub caller: Option<AccountId>,
/// The callee of the contract execution. Might be user or another contract.
///
/// We don't know the specifics of the AccountId ‒ like how many bytes or what
/// type of default `AccountId` makes sense ‒ they are left to be initialized
/// by the crate which uses the `engine`. Methods which require a callee might
/// panic when it has not been set.
/// We don't know the specifics of the AccountId ‒ like how many bytes or
/// what type of default `AccountId` makes sense ‒ they are left to be
/// initialized by the crate which uses the `engine`. Methods which
/// require a callee might panic when it has not been set.
pub callee: Option<AccountId>,
/// The value transferred to the contract as part of the call.
pub value_transferred: Balance,
Expand Down
Loading