Skip to content

Commit dad1921

Browse files
ggonzalez94qalisanderbidzyyys
authored
Use mini-alloc by default and remove panic handler (#373)
Mi intention is not that we directly merge this PR, but that it serves 2 purposes 1. Discuss how we want to handle global allocator and mainly what to do about the panic handler 2. In case we proceed wether we want to merge it to the release branch(and issue a new release) or just to main ## Background While testing how the experience will be for someone that creates a new stylus project by running `cargo stylus new` I realized a couple things that I think hurt DX. 1. You have to disable the default feature flags on the stylus sdk(which disables mini_alloc as the default globall allocator). Otherwise you get #global_allocator conflicts as it is defined in two places(the stylus sdk and our library). ``` toml stylus-sdk = { version = "=0.6.0", default-features = false } ``` 2. Since our library is `#[no_std]` by default it defines a custom panic_handler such that if you use it from a `no_std `environment you don’t have to define it yourself. The problem is that if you want to use the std library in your project(and that seems to be a reasonable assumption according to [this](https://github.com/OffchainLabs/stylus-sdk-rs/tree/main?tab=readme-ov-file#feature-highlights)), you cannot do it since it results in a duplication of the panic_handler function. This is also the default behavior when you create a new project with `cargo stylus new`. ## Proposed Solution 1. Using mini-alloc by default via the sdk seems like the best approach as it was one of the recommendations from the [audit](https://forum.arbitrum.foundation/t/stylus-sdk-audit/26674#p-58099-inefficient-allocator-fallback-in-stylus-contracts-44) and is what is used when creating a new project via `cargo stylus new`. 2. Here I see two options: - Add a compilation flag to the panic handler such that it is not included if built with std flag on. This is the approach I took in this PR, but it has 2 downsides: 1. If you just start a project from scratch using `cargo stylus new` it is harder to realize and you might spend some time trying to figure it out. 2. We are now encouraging people to use the `std` flag on our library, which might enable other behaviors we haven’t considered? Since we’ve been mainly using it for testing - Remove the panic handler entirely from the library, allowing projects building with std to have it included by default and projects explicitly opting in to `no_std` and having zero dependencies on the std library would have to define it themselves. This approach is more reasonable IMO, since it makes it easier for newcomers, but it requires some extra effort if you are experienced enough to define your contracts with `no_std` . The problem today is that we set the library type to [cdylib](https://doc.rust-lang.org/reference/linkage.html), which requires the crate to define a panic_handler when using `no_std` (more info [here](https://internals.rust-lang.org/t/panic-handler-free-no-std-targets/14697)). If we find a way to do this, maybe this is the best approach, maybe it is possible using unstable features. ## Edit - We were able to remove the panic handler entirely by not targeting `cdylib`(thanks @qalisander )! Now the user of the library can just use `std`or define their own panic handler. - [x] Test on a new stylus project - [x] Update the Readme - [x] Update Changelog --------- Co-authored-by: Alisander Qoshqosh <[email protected]> Co-authored-by: Alisander Qoshqosh <[email protected]> Co-authored-by: Daniel Bigos <[email protected]>
1 parent 7728693 commit dad1921

File tree

27 files changed

+94
-106
lines changed

27 files changed

+94
-106
lines changed

CHANGELOG.md

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
# Changelog
2+
3+
All notable changes to this project will be documented in this file.
4+
5+
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
6+
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
7+
8+
## [Unreleased]
9+
10+
### Added
11+
12+
-
13+
14+
### Changed
15+
16+
-
17+
18+
### Fixed
19+
20+
-
21+
22+
## [0.1.1] - 2024-10-
23+
24+
### Changed
25+
26+
- Mini alloc is now used by default via the stylus-sdk. This avoids conflicts with duplicate #[global_allocator] definitions. #373
27+
- Removed the panic handler from the library, making it easier for `std` and `no_std` projects to use the library. #373
28+
29+
## [0.1.0] - 2024-10-17
30+
31+
- Initial release

Cargo.lock

Lines changed: 26 additions & 62 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)