-
Notifications
You must be signed in to change notification settings - Fork 20
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
feat(zink): introduce derive macro contract #314
base: main
Are you sure you want to change the base?
Conversation
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.
the struct of the contract looks neat!
seems like the contract creating logic works however the dispatcher got problems, I'm not pretty sure about the root case of it, but I'd suggest create another example with simple logic for debugging it, e.g. a getter,
- make sure the getter functions work
- apply this macro to other examples and see if there will be new bugs introduced
- add TODOs for the calls like
_transfer
because we haven't solved it yet
examples/contract.rs
Outdated
#[global_allocator] | ||
static ALLOC: dlmalloc::GlobalDlmalloc = dlmalloc::GlobalDlmalloc; |
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.
what's the usage of the allocator here? if I recall correctly, we only need to on handling vector however we don't have the design of vector in zink
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.
the allocator is here because of SmallVec
. the runtime ERC20 logic doesn’t allocate on the heap, aligning with our design in zink, but the test setup triggers this requirement for wasm32 builds
zink/codegen/src/contract.rs
Outdated
use syn::{parse_macro_input, Fields, ItemStruct, Type}; | ||
|
||
/// Parse the input struct and generate the contract storage implementation | ||
pub fn parse(input: ItemStruct) -> TokenStream { |
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.
instead a single parse
function, I'd recommend to use a struct for making this more gracefully, for example, the implementation for the storage macro https://github.com/zink-lang/zink/blob/main/zink/codegen/src/storage.rs#L26
resolves #288
i made some choices i think you should be aware of
storage type choice:
Bytes32
OverString32
i opted for
Bytes32
for fields likename
andsymbol
instead ofString32
(aliased toU256
) due to API constraints inzink::primitives::U256
. initially, I attempted to useString32
to align with the original ERC20 intent:this failed because:
Bytes32::from_slice_unchecked
is not available inzink::primitives::Bytes32
(onlyempty()
andeq()
exist perimpl_bytes!
inbytes.rs
)U256 Private Field
: U256(Bytes32) constructor isn’t public (pub struct U256(Bytes32)
has a private field), and nofrom_big_endian
orfrom_little_endian methods
exist—onlyFrom<u64>
,empty()
, andmax()
are available.switching to Bytes32 resolved this:
now,
Bytes32
directly wraps[u8; 32]
in non-WASM (viaimpl_bytes!
), and the macro generatesERC20Name::get()
as a simplesload
, expecting a Bytes32 value. this avoidsU256
’s construction issues and matches the storage slot format ([u8; 32]
).bypassing contract calls to avoid revert
the test (
test_storage
) uses directevm.storage()
reads instead of contract calls likename()
:this was necessary because runtime calls consistently reverted:
revert Cause: the message "Empty from address" originates from _transfer:
unexpected trigger:
name()
(self.name() -> ERC20Name::get()) is a storage read (sload at slot 0), yet it reverts with this message. which i think (not sure) is a dispatcher issue in zinkc.direct
evm.storage()
readsslot 0 (name)
,1 (symbol)
, and2 (total_supply)
without WASM execution, confirming the macro’sconstruct()
sets storage correctly. this isolates the problem to runtime dispatch, not storage setup.the runtime revert is a blocker for testing core logic (
transfer(), approve()
). It seemszinkc
misroutes external calls (e.g.,name()
selector0x06fdde03
) to_transfer (0xa9059cbb)
, unlike erc20.rs where separate structs work fine. could this be:a dispatcher bug in selector::external or zinkc bytecode generation?
a problem with unified structs vs. separate ones?
i’d love your take on this—any ideas on why
_transfer
is hijacking calls? maybe a peek at the WASM output or dispatcher logic could shed light?i’ve kept the test minimal to prove storage, but i really want to get runtime calls working.