OpenZeppelin conducted a diff audit of the OffchainLabs/stylus-sdk-rs repository between the base commit f56fc6b and the head commit 0d20340.
In scope were the following files:
.
├── examples
│ ├── erc20
│ │ └── src
│ │ └── main.rs
│ ├── erc721
│ │ └── src
│ │ └── main.rs
│ └── single_call
│ └── src
│ ├── lib.rs
│ └── main.rs
├── stylus-core
│ └── src
│ ├── calls
│ │ ├── context.rs
│ │ ├── errors.rs
│ │ └── mod.rs
│ ├── deploy.rs
│ ├── host.rs
│ ├── lib.rs
│ └── storage.rs
├── stylus-proc
│ ├── src
│ │ ├── consts.rs
│ │ ├── lib.rs
│ │ ├── macros
│ │ │ ├── derive
│ │ │ │ └── erase.rs
│ │ │ ├── entrypoint.rs
│ │ │ ├── public
│ │ │ │ ├── attrs.rs
│ │ │ │ ├── export_abi.rs
│ │ │ │ ├── mod.rs
│ │ │ │ ├── overrides.rs
│ │ │ │ └── types.rs
│ │ │ ├── sol_interface.rs
│ │ │ └── storage.rs
│ │ └── types.rs
│ └── tests
│ ├── derive_erase.rs
│ ├── fail
│ │ └── public
│ │ └── constructor.rs
│ ├── public.rs
│ └── public_composition.rs
├── stylus-sdk
│ └── src
│ ├── abi
│ │ ├── bytes.rs
│ │ ├── const_string.rs
│ │ ├── export
│ │ │ └── mod.rs
│ │ ├── impls.rs
│ │ ├── internal.rs
│ │ ├── ints.rs
│ │ └── mod.rs
│ ├── call
│ │ ├── context.rs
│ │ ├── error.rs
│ │ ├── mod.rs
│ │ ├── raw.rs
│ │ ├── traits.rs
│ │ └── transfer.rs
│ ├── contract.rs
│ ├── deploy
│ │ ├── mod.rs
│ │ └── raw.rs
│ ├── evm.rs
│ ├── host
│ │ ├── calls.rs
│ │ ├── deploy.rs
│ │ └── mod.rs
│ ├── hostio.rs
│ ├── lib.rs
│ ├── prelude.rs
│ ├── storage
│ │ ├── array.rs
│ │ ├── bytes.rs
│ │ ├── map.rs
│ │ ├── mod.rs
│ │ ├── traits.rs
│ │ └── vec.rs
│ ├── tx.rs
│ └── types.rs
└── stylus-test
└── src
├── builder.rs
├── constants.rs
├── lib.rs
├── state.rs
└── vm.rs
In addition, pull request #131 of the OffchainLabs/cargo-stylus repository was audited.
In scope were the following files:
.
└── main/src
├── deploy
│ ├── deployer.rs
│ └── mod.rs
├── export_abi.rs
└── main.rs
Finally, the following file from pull request #294 of the OffchainLabs/nitro-contracts repository was also audited:
src/stylus
└── StylusDeployer.sol
This release includes three key changes and features:
Previous versions of Stylus used the hostio
module to facilitate interactions between smart contracts and the host environment. This update introduces the new Host
trait to the SDK, following the plan to move away from previous global hostio
invocations and instead making it possible for Stylus programs to access a host environment using a convenient self
-method, making the SDK a lot simpler and clearer.
The Stylus SDK parametrizes contracts with a host trait, which allows developers to use a mock host trait for unit testing. To ease the testing implementation, the Stylus SDK now implements a TestVM
that can be passed to a contract's instance in a test to mock the functionality of the host. The TestVM
offers an API to customize the host, for instance, changing the balance of an address or the message sender, updating storage slots, mocking calls, reading block data, and more.
This new release of the Stylus SDK adds support for smart contract constructors, another feature that was missing in previous versions. This feature was implemented in three different repositories.
First, the OffchainLabs/stylus-sdk-rs repository added a new constructor macro that allows contracts to define a function as a constructor. The constructor behaves similarly to an initializer function. This means that the constructor function needs to be called after deployment and is not called during deployment like its Solidity counterpart. Once the constructor is invoked, a mechanism makes sure to prevent further calls to the constructor.
Then, since the contract deployment and constructor calling happen in two different steps, i.e., in a non-atomical manner, the StylusDeployer
contract was added to the OffchainLabs/nitro-contracts repository. This contract helps deploy and call the Stylus constructor in a single transaction, allowing it to execute a Stylus contract deployment and initialization atomically.
Lastly, the OffchainLabs/cargo-stylus repository was modified to support contract deployment by adding optional constructor parameters to the deploy
command. This method will leverage the StylusDeployer
contract described previously to initiate the deployment and initialization transaction.
The new inheritance model introduced in the Stylus SDK aims to transition from Solidity-style multiple inheritance patterns toward a more idiomatic Rust approach centered around composition and trait-based polymorphism. Rather than relying on the #[inherit]
and #[borrow]
macros to simulate linear inheritance chains, this design promotes the encapsulation of logic within modular components, each exposing a trait that defines external-facing behavior.
While #[inherit]
and #[borrow]
remain available in the SDK and may be effective solutions for simple inheritance, the new changes introduce the #[implements(...)]
macro which may include any number of traits as arguments for which the top-level storage contract (i.e., the storage definition with #[entrypoint]
macro, representing the main contract within a crate) must implement. Contracts can then compose multiple such components by embedding them as fields in the entry point struct and implementing their associated traits.
This allows for greater flexibility in reuse, clearer boundaries between storage and logic, and improved compatibility with Rust’s type system. The routing of external calls continues to be handled by the SDK's Router
trait, which has been refactored to support trait-based dispatch across multiple #[public]
blocks. This approach is intended to reduce complexity, improve maintainability, and align more closely with Rust’s strengths in safety and modularity.
Stylus refuses reentrancy by default. To enable reentrancy, a user must explicitly enable the reentrant
feature when importing stylus-sdk
. If a contract does not allow reentrancy, the deny_reentrant
logic will be added right after the user's entry point, and will revert if the message is flagged as reentrant.
However, the user_entrypoint
function is only defined when contracts are compiled to the wasm32
target, which is not the case when testing. Moreover, the reentrant
field in the VMState
is hardcoded to false
and cannot be changed in TestVM
. This means that contracts cannot prevent reentrancy when testing, resulting in potentially unexpected test results and making the reentrancy feature impossible to test.
Consider adding a mechanism for contracts compiled for a non-wasm32
VM to revert on reentrant calls making Stylus' security platform independent and future-proof in case of changes to the underlying infrastructure.
Update: Acknowledged, not resolved. The Offchain Labs team stated:
We are continuing to take feedback for improvements in the new testing framework. We are also looking at reentrancy due to unrelated findings.
fallback
and receive
Functions Not Called for Trait-Based Inheritancefallback
and receive
are special functions which are executed directly on a contract, without first invoking its route
function. In case a contract does not have its own fallback
or receive
function, it can inherit them from one of the parent contracts. However, this logic only applies when the #[inherit(...)]
attribute is used for inheritance.
This means that if a contract uses trait-based inheritance and a fallback
or receive
function is defined through any of the traits listed in the #[implements(...)]
attribute, they will not be callable. This could be surprising for the developers, because in Solidity, an inheriting contract exposes fallback
and receive
functions defined in a parent contract, and for many contracts (e.g., ERC1967Proxy
), this behavior is crucial so that they can function correctly.
Consider calling the fallback
and receive
functions of parent contracts defined in the #[implements(...)]
attribute in case they are not implemented in the inheriting contract.
Update: Resolved in pull request #241 at commit 18469c1. The Offchain Labs team stated:
We ended up deciding to remove
fallback
/receive
inheritance. We believe that the semantics of inheritance, especially trait-based composition, becomes quite complicated withfallback
/receive
functions, and decided to do as follows: contracts can definefallback
/receive
methods. If they do not, a default will be provided. While this does not match Solidity, we believe that it improves the simplicity of that code path in the SDK and makes contracts more auditable.
In the case of inheritance performed with the #[inherit(...)]
attribute, there are override checks inserted into the code in order to ensure that the inheriting contract does not override any function with a function with a less restrictive purity. However, similar checks are not performed in case when the trait-based inheritance is used, which means that it is possible to override a function from a trait listed in the #[implements(...)]
attribute with another one that has a less restrictive purity.
Another case exists that can bypass mutability rules when a contract combines both the #[implements(...)]
and #[inherit(...)]
attributes. Since implements_routes
comes before inheritance_routes
in the generated code, functions with matching selectors from trait-based inheritance will be prioritized during function call routing. Therefore, in the case where functions with the same selector exist in both a parent from #[implements(...)]
and #[inherit(...)]
then a function from trait-based inheritance can override a function from a parent inherited through #[inherit(...)]
with less restrictive purity.
Consider performing override checks for trait-based inheritance and validating that any shared selectors between implement routes and inherit routes follow correct mutability rules.
Update: Acknowledged, will resolve. The Offchain Labs team stated:
We are continuing to take feedback for improvements in trait-based inheritance. We will be sure to consider how overriding may affect the routing development and auditability.
The addition of trait-based inheritance currently contains a single test in the [public_composition99299299](https://github.com/OffchainLabs/stylus-sdk-rs/blob/0d20340efd04191379d21138f646d9960303e5cb/stylus-proc/tests/public_composition.rs) which only provides a simple example using the
#[implements(...)]` macro and exists only to check that a contract using this macro can compile successfully. This limited test coverage may lead to undetected issues and hinder the verification of code functionality across various modules.
To ensure that the new inheritance model functions correctly under different scenarios, consider adding at least one more example in the src/examples
folder to test behavior such as combining #[inherit(...)]
and #[implements(...)]
, expected results of inherited #[fallback]
and #[receive]
, and combining with other functionalities within the system. In addition, consider providing unit tests using the stylus-test
crate to illustrate its usage while providing more robust coverage to different parts of the SDK.
Update: Acknowledged, will resolve. The Offchain Labs team stated:
We are working on improving the testing coverage for this feature as we iterate on it.
The Stylus SDK is designed to allow contract constructors if a function is annotated with the #[constructor]
attribute. The constructor function can be of any name and it will always carry the constructor selector. The constructor will be called only once when the contract is deployed.
However, a contract can have both a function with the #[constructor]
annotation and another function with the selector changed to match the constructor using #[selector(name = "stylus_constructor")]
. This will cause the function with the custom selector to become unreachable since the constructor gets priority in the routing logic. This collision could be potentially exploited by providing a public ABI to a smart contract function that will actually call the stylus constructor (which will revert on any call except the first).
Consider disallowing stylus_constructor
as a potential argument when providing custom selectors.
Update: Resolved in pull request #242 at commit 217a92c.
tx_origin
Value in VMState
The default tx_origin
in the stylus_test::state
crate is set to Address::ZERO
. This could be confusing because the tx_origin
value should be identical to the msg_sender
value when initiating a transaction. This could lead to unexpected test results.
Consider defaulting tx_origin
to DEFAULT_SENDER
to match msg_sender
's default value.
Update: Resolved in pull request #241 at commit ed2a1d4.
TestVMBuilder
To construct a TestVM
, a user can either call TestVM::default()
or build the custom TestVM
using the TestVMBuilder
, which gives the user the options to set custom parameters like message sender, message value, the contract's address, and the RPC URL. When building a custom TestVM
using the TestVMBuilder::build
function, the custom parameters will be set in the TestVM
as previously defined.
However, certain fields are not set to the same default value. For instance, if a user does not define a custom contract_address
, the value will be set to the Address::ZERO
instead of being set to the default DEFAULT_CONTRACT_ADDRESS
value. This behavior creates inconsistencies with the default VM created using TestVM::default
.
Consider setting the values of the uncustomized fields to the same values in VMState::default
to enhance consistency and avoid unexpected test results.
Update: Resolved in pull request #241 at commit ed2a1d4 where TestVM
will use default states and override them with any specific changes.
The DeploymentAccess
trait in the stylus_core::deploy
crate defines two deploy
functions. Both functions return the Address
type when they succeed and an error of Vec
type on failure. The DeploymentAccess
implementation for TestVM
implements both deploy
functions with the same signature. The deploy
functions are implemented to mimic an actual deploy call, by attempting to get the address stored in the VM_State.deploy_returns
hash map at the DeploymentWithSalt
key. If no address is found, it is assumed that the deployment failed. However, there is no way to test a deployment failure, since both functions default to Address::ZERO
[1] [2] when no address is found instead of returning an error.
Consider returning an error from the deploy
functions implemented in the TestVM
to allow users to test deployment failures.
Update: Partially resolved in pull request #241 at commit ed2a1d4, where deploy
will panic if anything goes wrong but does not handle errors.
TestVM
The stylus_test::vm
module defines the TestVM
environment for unit-testing Stylus contracts. The TestVM
implements the necessary functions and traits to allow for the mocking of host methods defined on the [stylus_core::host::Host
] trait, such as access to storage, messages, block values, and more.
However, the tests in this module are currently limited. Since the TestVM
serves as the foundation for testing Stylus contracts, it is crucial to ensure that its state behaves as expected when users modify it while writing tests. Maintaining the integrity of the testing framework is essential, yet important functions like set_sender
, set_value
, and others remain untested.
To improve test coverage and reliability, consider adding additional tests for these functions to verify their behavior and ensure the correctness of the TestVM
state.
Update: Resolved in pull request #241 at commit ed2a1d4.
VMState
and TestVMBuilder
The chain_id
field is hardcoded in the VMState
and cannot be changed in the TestVM
. This could make testing signatures or replay attacks that depend on a Chain ID that is different from DEFAULT_CHAIN_ID
impossible (e.g., on the Arbitrum testnet).
The stylus_test
crate offers a TestVMBuilder
struct as a way to create TestVM
from selected values. These values can be modified with the methods provided by the builder pattern. However, some of the defined values do not have setter methods in this process (e.g., storage
and block_number
).
Consider making the chain_id
field customizable to further enhance the testing capabilities of the Stylus SDK. In addition, consider providing relevant methods for the above-mentioned fields of the TestVMBuilder
struct or removing them entirely and using default values instead.
Update: Resolved in pull request #241 at commit ed2a1d4.
The Stylus SDK includes a feature to generate an ABI for a contract in the form of a Solidity interface. The cargo stylus export-abi
command normally generates an interface for each #[public]
implementation block. However, when the trait_
property exists, the Extension::codegen
step is skipped. This corresponds to blocks for trait implementations, which may also appear in an #[inherit(...)]
attribute. Hence, no Solidity interface will appear in the exported ABIs for trait implementation blocks.
Consider adding logic to export a Solidity interface for #[public]
trait implementations while having the Solidity interfaces generated from a block with the #[inherit(...)]
macro actually extending the inherited interface.
Update: Acknowledged, will resolve. The Offchain Labs team stated:
This has been noted for upcoming development on the export abi functionality.
todo!()
in codegen
for Unsupported self_ty
VariantsThe codegen
function in macros/public/export_abi.rs of stylus-proc crate uses the todo!()
macro for unsupported self_ty
types, which indicates that handling for other types has not been implemented. If the function encounters a self_ty
type that does not match the expected variant, it will result in a compile-time panic, potentially causing unexpected behavior.
Consider providing a clear error message or implementing a default case to handle unsupported self_ty
types. Alternatively, consider expanding the support for additional types.
Update: Acknowledged, will resolve. The Offchain Labs team stated:
This should not be reachable and would only result in a compile-time error. We are considering how to emit a more useful error for these cases if necessary.
In line 153 of mod.rs
, the from()
associated function implemented for PublicFn
checks for special function names that are not defined with associated attributes. If triggered, this function will emit an error. However, this process does not consider case-sensitive situations like Constructor
, Fallback
, etc., which could lead to missing special named functions.
Consider adding a case formatting before the comparison as shown below:
if matches!(kind, FnKind::Function) && name.to_string().to_lowercase() == special_name { ... }
Update: Resolved in pull request #241 at commit ed2a1d4.
ink_to_gas
The ink_to_gas
function in the src/host.rs
file from the Stylus core crate divides ink
by self.tx_ink_price()
, cast to u64
. If tx_ink_price()
returns zero, this results in a division-by-zero error. This would cause a runtime panic in Rust, leading to contract failure and potentially halting transaction processing. Arbitrum enforces a minimum gas price floor, which significantly reduces the likelihood of tx_ink_price()
returning zero.
Consider adding a check such as if self.tx_ink_price() == 0 { return 0; }
before the division to handle this error gracefully. Alternatively, consider ensuring and documenting that the tx_ink_price()
never returns zero.
Update: Resolved in pull request #241 at commit ed2a1d4.
Throughout the codebase, multiple instances of misleading documentation were identified:
stylus_core::calls
should also mention that this trait can be used for pure
functions.stylus_core::host
should advise using flush_cache
instead of storage_flush_cache
.stylus_test::builder
should say vm.set_value(value)
instead of vm.set_msg_value(value)
.Furthermore, the codebase lacks sufficient documentation and could greatly benefit from a thorough review and improvement. This is specifically the case for sensitive functions. For instance, in stylus_proc::macros::entrypoint
, the mark_used_fn
and user_entrypoint_fn
functions are not documented. In addition, the codebase only includes the documentation for testing smart contracts, while no documentation for test implementation for bytes in, bytes out functions has been provided.
Consider enhancing the documentation, particularly for key areas of the codebase, to improve its clarity and usability.
Update: Partially resolved in pull request #241 at commit ed2a1d4, where the above-listed three instances were fixed, but sufficient documentation on certain functions is still missing.
fallback
and receive
Functions in Parent Contracts May Be Silently OverriddenWhen using the #[inherit(...)]
macro for contract inheritance in Stylus, multiple parent contracts can define their own fallback
and receive
functions. If the child contract does not explicitly define these functions, the current implementation will default to using the first occurrence found among the parents.
This diverges from Solidity’s inheritance model, where the compiler enforces that a child contract must explicitly override fallback
or receive
functions if they are defined in multiple parent contracts. The Stylus behavior can lead to unexpected and potentially dangerous outcomes: some fallback
or receive
implementations may become silently unreachable, even if they are critical to the correct functioning of a parent contract.
To mitigate this risk, consider requiring child contracts to explicitly override fallback
and receive
functions when multiple parent implementations exist. This would make the behavior explicit and align with developer expectations established by Solidity. Additionally, similar safeguards should be applied to trait-based inheritance patterns. At a minimum, the current behavior should be clearly documented to prevent developer confusion and accidental misconfiguration.
Update: Acknowledged, will resolve. The Offchain Labs team stated:
We are planning to deprecate and remove the old inheritance model, so we do not plan to make changes here.
print_from_args
The print_from_args
function in the stylus_sdk::abi::export
crate is used to print either a contract's ABI or its constructor's signature based on the CLI input. When printing the ABI, users have two options: they can either provide a custom license
and pragma
or use the default values (DEFAULT_LICENSE
and DEFAULT_PRAGMA
).
Currently, the function does not allow users to define either the license
or the pragma
. Instead, users must either specify both values or rely on defaults for both. This limitation can create unnecessary friction for developers, as they are forced to provide both values even when only one needs customization.
To improve usability and ease adoption, consider modifying the print_from_args
function to allow users to define either the license
or the pragma
independently, while relying on the default value for the undefined argument.
Update: Acknowledged, not resolved. The Offchain Labs team stated:
The user can customize each one independently by setting the corresponding CLI args. If the arg is not set, it will use the default value. We also allow the user to not pass the abi command (for retro-compatibility) and use the default values.
self.state
Within the storage_load_bytes32
function in stylus_test::vm
, self.state
is borrowed three times. Since self.state
is of type RefCell
, every borrow creates overhead checks at runtime.
To improve code efficiency, consider refactoring the storage_load_bytes32
function by caching the value of the current state at the beginning of the function and reusing it later.
Update: Resolved in pull request #241 at commit ed2a1d4.
rpc_url
The rpc_url
function in stylus_test::builder
checks if self.rpc_url
is Some
on line 67. However, rpc_url
gets wrapped in the previous line, rendering this check redundant. Moreover, the code is calling unwrap
on the parsed URL which is a user input in line 68 which could lead to a panic.
Consider removing the redundant check to enhance code efficiency and refining error handling in URL parsing to provide clearer error messages for the user.
Update: Resolved in pull request #241 at commit ed2a1d4.
The storage_load_bytes32
function in line 357 of the vm.rs
file in the stylus-test crate clones the provider and uses it to access storage
if let Some(provider) = self.state.borrow().provider.clone() {
However, it can use a reference instead to save a clone to improve the efficiency of the codebase.
if let Some(provider) = &self.state.borrow().provider {
Consider refactoring the code to use a reference instead.
Update: Resolved in pull request #241 at commit ed2a1d4, where this code was removed as part of the fix for another issue.
Throughout the codebase, multiple instances of typographical errors were identified:
stylus_sdk::utils
, replace the "g" with a space between "see" and "https://".stylus_core::host
.stylus_core::host
.stylus_sdk::storage::map
.#[implements(Parent1, Parent2]
" should be "#[implements(Parent1, Parent2)]
" in line 27 in stylus_proc::macros::public::attrs
.Consider correcting the aforementioned typographical errors and checking the codebase thoroughly for any other ones before deploying.
Update: Acknowledged, will resolve.
Throughout the codebase, multiple instances of configurations that are either improperly placed or redundant were identified:
stylus_core::calls::context
, any
in the configuration attribute is redundant because it only has one child. Since any
returns true
if any of its children evaluates to true
, and there is only one child, the any
check can be safely removed without changing the behavior. This simplifies the code and improves readability.#[allow(unused)]
lint attribute in line 48 in stylus_core::calls::errors
should only be placed over the second arm to avoid unintentionally leaving a variable unused.Consider implementing the above-given recommendations to improve the clarity and readability of the codebase.
Update: Resolved in pull request #241 at commit ed2a1d4.
The newly introduced inheritance model involves the addition of the #[implements(...)]
attribute. However, this attribute's name is not consistent with the #[inherit(...)]
attribute used for inheritance so far, as it contains an "s" at the end. During testing, we noticed it was easy to forget this distinction between the two. As a result, it could confuse developers who may expect that the attributes will be named in a consistent manner. Consider making the attributes' names consistent with each other, either by renaming the #[implements(...)]
attribute to #[implement(...)]
or by renaming the #[inherit(...)]
attribute to #[inherits(...)]
.
Furthermore, an inconsistency was identified in the naming of variables during parsing of these attributes. When parsing inherits
, the variable within Some
is named inherits
, whereas when parsing implements
, the corresponding variable is named attr
. Consider renaming the attr
variable to something closer to implements
to improve consistency and facilitate quicker manual parsing of the code.
Consider implementing the aforementioned renaming suggestions to improve the clarity and readability of the codebase.
Update: Acknowledged, will resolve. The Offchain Labs team stated:
This issue will be resolved once the old inheritance model is removed.
The latest Stylus SDK release introduces a range of powerful features that improve usability, modularity, and testing support. These include a mock host testing framework (TestVM
), support for contract constructors critical for initialization, and a new trait-based inheritance model designed to promote Rust-style composition over Solidity-style linear inheritance. These improvements significantly enhance the development experience, testability, and code organization for Stylus-based smart contracts.
However, the new inheritance model introduces important edge cases and behavioral inconsistencies—particularly with fallback handling and purity override checks that differ from Solidity expectations. These inconsistencies, along with the constructor naming flexibility, could lead to confusion or misbehavior in real-world contracts if left unaddressed.
While the update represents meaningful progress, it is recommended that the Stylus team continues refining the new inheritance model and expands testing coverage, particularly for constructors within TestVM
, to ensure reliable behavior. Currently, key constructor-related functions like state initialization lack sufficient tests, risking unexpected outcomes in deployment scenarios.