-
Notifications
You must be signed in to change notification settings - Fork 450
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
Allow option to opt-out of provided memory allocator #1661
Merged
Merged
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
eddcb1f
Add feature to disable global memory allocator
HCastano c7a3203
Add example of how to use a different allocator
HCastano 88f06ef
Add concrete downsides to warning
HCastano 8631c14
Update example to use a `Vec`
HCastano a305703
Appease Clippy
HCastano 74dd773
Add missing period
HCastano 99970a3
Merge branch 'master' into hc-no-allocator
HCastano ebbac25
Move example to `integration-tests` folder
HCastano File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,3 +25,4 @@ quickcheck_macros = "1" | |
default = ["std"] | ||
std = [] | ||
ink-fuzz-tests = ["std"] | ||
no-allocator = [] |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
# Ignore build artifacts from the local tests sub-crate. | ||
/target/ | ||
|
||
# Ignore backup files creates by cargo fmt. | ||
**/*.rs.bk | ||
|
||
# Remove Cargo.lock when creating an executable, leave it for libraries | ||
# More information here http://doc.crates.io/guide.html#cargotoml-vs-cargolock | ||
Cargo.lock |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
[package] | ||
name = "custom_allocator" | ||
version = "4.0.0" | ||
authors = ["Parity Technologies <[email protected]>"] | ||
edition = "2021" | ||
publish = false | ||
|
||
[dependencies] | ||
# We're going to use a different allocator than the one provided by ink!. To do that we | ||
# first need to disable the included memory allocator. | ||
ink = { path = "../../crates/ink", default-features = false, features = ["no-allocator"] } | ||
|
||
# This is going to be our new global memory allocator. | ||
dlmalloc = {version = "0.2", default-features = false, features = ["global"] } | ||
|
||
scale = { package = "parity-scale-codec", version = "3", default-features = false, features = ["derive"] } | ||
scale-info = { version = "2.3", default-features = false, features = ["derive"], optional = true } | ||
|
||
[dev-dependencies] | ||
ink_e2e = { path = "../../crates/e2e" } | ||
|
||
[lib] | ||
path = "lib.rs" | ||
|
||
[features] | ||
default = ["std"] | ||
std = [ | ||
"ink/std", | ||
"scale/std", | ||
"scale-info/std", | ||
] | ||
ink-as-dependency = [] | ||
e2e-tests = [] |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,180 @@ | ||
//! # Custom Allocator | ||
//! | ||
//! This example demonstrates how to opt-out of the ink! provided global memory allocator. | ||
//! | ||
//! We will use [`dlmalloc`](https://github.com/alexcrichton/dlmalloc-rs) instead. | ||
//! | ||
//! ## Warning! | ||
//! | ||
//! We **do not** recommend you opt-out of the provided allocator for production contract | ||
//! deployments! | ||
//! | ||
//! If you don't handle allocations correctly you can introduce security vulnerabilities to your | ||
//! contracts. | ||
//! | ||
//! You may also introduce performance issues. This is because the code of your allocator will | ||
//! be included in the final contract binary, potentially increasing gas usage significantly. | ||
//! | ||
//! ## Why Change the Allocator? | ||
//! | ||
//! The default memory allocator was designed to have a tiny size footprint, and made some | ||
//! compromises to achieve that, e.g it does not free/deallocate memory | ||
HCastano marked this conversation as resolved.
Show resolved
Hide resolved
|
||
//! | ||
//! You may have a use case where you want to deallocate memory, or allocate it using a different | ||
//! strategy. | ||
//! | ||
//! Providing your own allocator lets you choose the right tradeoffs for your use case. | ||
|
||
#![cfg_attr(not(feature = "std"), no_std)] | ||
// Since we opted out of the default allocator we must also bring our own out-of-memory (OOM) | ||
// handler. The Rust compiler doesn't let us do this unless we add this unstable/nightly feature. | ||
#![cfg_attr(not(feature = "std"), feature(alloc_error_handler))] | ||
|
||
// Here we set `dlmalloc` to be the global memory allocator. | ||
// | ||
// The [`GlobalAlloc`](https://doc.rust-lang.org/std/alloc/trait.GlobalAlloc.html) trait is | ||
// important to understand if you're swapping our your allocator. | ||
#[cfg(not(feature = "std"))] | ||
#[global_allocator] | ||
static ALLOC: dlmalloc::GlobalDlmalloc = dlmalloc::GlobalDlmalloc; | ||
|
||
// As mentioned earlier, we need to provide our own OOM handler. | ||
// | ||
// We don't try and handle this and opt to abort contract execution instead. | ||
#[cfg(not(feature = "std"))] | ||
#[alloc_error_handler] | ||
fn oom(_: core::alloc::Layout) -> ! { | ||
core::arch::wasm32::unreachable() | ||
} | ||
|
||
#[ink::contract] | ||
mod custom_allocator { | ||
use ink::prelude::{ | ||
vec, | ||
vec::Vec, | ||
}; | ||
|
||
#[ink(storage)] | ||
pub struct CustomAllocator { | ||
/// Stores a single `bool` value on the storage. | ||
/// | ||
/// # Note | ||
/// | ||
/// We're using a `Vec` here as it allocates its elements onto the heap, as opposed to the | ||
/// stack. This allows us to demonstrate that our new allocator actually works. | ||
value: Vec<bool>, | ||
} | ||
|
||
impl CustomAllocator { | ||
/// Constructor that initializes the `bool` value to the given `init_value`. | ||
#[ink(constructor)] | ||
pub fn new(init_value: bool) -> Self { | ||
Self { | ||
value: vec![init_value], | ||
} | ||
} | ||
|
||
/// Creates a new flipper smart contract initialized to `false`. | ||
#[ink(constructor)] | ||
pub fn default() -> Self { | ||
Self::new(Default::default()) | ||
} | ||
|
||
/// A message that can be called on instantiated contracts. | ||
/// This one flips the value of the stored `bool` from `true` | ||
/// to `false` and vice versa. | ||
#[ink(message)] | ||
pub fn flip(&mut self) { | ||
self.value[0] = !self.value[0]; | ||
} | ||
|
||
/// Simply returns the current value of our `bool`. | ||
#[ink(message)] | ||
pub fn get(&self) -> bool { | ||
self.value[0] | ||
} | ||
} | ||
|
||
#[cfg(test)] | ||
mod tests { | ||
use super::*; | ||
|
||
#[ink::test] | ||
fn default_works() { | ||
let custom_allocator = CustomAllocator::default(); | ||
assert!(!custom_allocator.get()); | ||
} | ||
|
||
#[ink::test] | ||
fn it_works() { | ||
let mut custom_allocator = CustomAllocator::new(false); | ||
assert!(!custom_allocator.get()); | ||
custom_allocator.flip(); | ||
assert!(custom_allocator.get()); | ||
} | ||
} | ||
|
||
#[cfg(all(test, feature = "e2e-tests"))] | ||
mod e2e_tests { | ||
use super::*; | ||
|
||
use ink_e2e::build_message; | ||
|
||
type E2EResult<T> = std::result::Result<T, Box<dyn std::error::Error>>; | ||
|
||
/// We test that we can upload and instantiate the contract using its default constructor. | ||
#[ink_e2e::test] | ||
async fn default_works(mut client: ink_e2e::Client<C, E>) -> E2EResult<()> { | ||
// Given | ||
let constructor = CustomAllocatorRef::default(); | ||
|
||
// When | ||
let contract_account_id = client | ||
.instantiate("custom_allocator", &ink_e2e::alice(), constructor, 0, None) | ||
.await | ||
.expect("instantiate failed") | ||
.account_id; | ||
|
||
// Then | ||
let get = build_message::<CustomAllocatorRef>(contract_account_id.clone()) | ||
.call(|custom_allocator| custom_allocator.get()); | ||
let get_result = client.call_dry_run(&ink_e2e::alice(), &get, 0, None).await; | ||
assert!(matches!(get_result.return_value(), false)); | ||
|
||
Ok(()) | ||
} | ||
|
||
/// We test that we can read and write a value from the on-chain contract contract. | ||
#[ink_e2e::test] | ||
async fn it_works(mut client: ink_e2e::Client<C, E>) -> E2EResult<()> { | ||
// Given | ||
let constructor = CustomAllocatorRef::new(false); | ||
let contract_account_id = client | ||
.instantiate("custom_allocator", &ink_e2e::bob(), constructor, 0, None) | ||
.await | ||
.expect("instantiate failed") | ||
.account_id; | ||
|
||
let get = build_message::<CustomAllocatorRef>(contract_account_id.clone()) | ||
.call(|custom_allocator| custom_allocator.get()); | ||
let get_result = client.call_dry_run(&ink_e2e::bob(), &get, 0, None).await; | ||
assert!(matches!(get_result.return_value(), false)); | ||
|
||
// When | ||
let flip = build_message::<CustomAllocatorRef>(contract_account_id.clone()) | ||
.call(|custom_allocator| custom_allocator.flip()); | ||
let _flip_result = client | ||
.call(&ink_e2e::bob(), flip, 0, None) | ||
.await | ||
.expect("flip failed"); | ||
|
||
// Then | ||
let get = build_message::<CustomAllocatorRef>(contract_account_id.clone()) | ||
.call(|custom_allocator| custom_allocator.get()); | ||
let get_result = client.call_dry_run(&ink_e2e::bob(), &get, 0, None).await; | ||
assert!(matches!(get_result.return_value(), true)); | ||
|
||
Ok(()) | ||
} | ||
} | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lemme be the guy commenting about something on the order of 80 CPL 😬 .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we configure
rustfmt
to catch this? I think it enforces code CPL to 100, not sure about the comments.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already enforce this to 90 characters, which is what it's at 😉
https://github.com/paritytech/ink/blob/b8862a1147a8fb577c82304ed1dec9a8c45c386a/.rustfmt.toml#L1
Edit:
Actually, we should be enforcing comments as 80
https://github.com/paritytech/ink/blob/b8862a1147a8fb577c82304ed1dec9a8c45c386a/.rustfmt.toml#L9
So maybe there is something to look into here 🤔
But everything else is under 100 which is reasonable imo.