feat(zink-codegen): introduce derive macro for events #281

malik672 · 2024-11-26T18:09:20Z

Resolves #255

generate ABIs for events and gathered events as enums
introduce util function for converting type to string

clearloop · 2024-11-27T04:26:29Z

Hi @malik672, could you please follow conventional commits in your commit messages, for example for the title

# zink is the main package you modified
feat(zink-codegen): introduce derive macro for events

and add details for what you modified in a specific commit message

# @4977bd5
-  feat(zink): generate ABIs for events
# @b23c12a
- feat(zink): introduce util function for converting type to string

that would be helpful for reviewing, thank you!

malik672 · 2024-11-27T05:52:47Z

Hi @malik672, could you please follow conventional commits in your commit messages, for example for the title
# zink is the main package you modified
feat(zink-codegen): introduce derive macro for events 
and add details for what you modified in a specific commit message
# @4977bd5
-  feat(zink): generate ABIs for events
# @b23c12a
- feat(zink): introduce util function for converting type to string
that would be helpful for reviewing, thank you!

done

clearloop · 2024-11-27T06:07:48Z

done

Oh sorry, I mean in your commit messages but not the description

I'll help you edit the description, you just need to use the conventional commit messages in your future commits

NOTE

when you complete this PR, please refactor the log example and make sure the CI is green

https://github.com/zink-lang/zink/blob/main/examples/log.rs#L10

btw please add your polygon address in the description of this PR, I'll transfer the budget to you as soon as the PR merged!

malik672 · 2024-11-27T06:50:39Z

done

Oh sorry, I mean in your commit messages but not the description

I'll help you edit the description, you just need to use the conventional commit messages in your future commits

NOTE

when you complete this PR, please refactor the log example and make sure the CI is green

https://github.com/zink-lang/zink/blob/main/examples/log.rs#L10

btw please add your polygon address in the description of this PR, I'll transfer the budget to you as soon as the PR merged!

apart from the log refactor is anything still remaining for the PR ?

clearloop · 2024-11-27T08:25:00Z

apart from the log refactor is anything still remaining for the PR ?

The test is the main thing! because our purpose is introducing a nice developer interface ^ ^

#[derive(Event)]
pub enum ERC20Events {
   Transfer(from: Address, to: Address, value: U256),
   Approval(owner: Address, spender: Address, value: U256),
   // ...
}

#[zink::external]
fn foo(...) {
   ERC20Events::Transfer(from, to, value).emit()
}

If the described interface works, our PR works, we may encounter issues while implementing this interface, and they're the problems we need to solve in this PR, please pay attention to the inline document and the handbook as well since it will help others using our code!

malik672 · 2024-11-27T08:42:17Z

apart from the log refactor is anything still remaining for the PR ?

The test is the main thing! because our purpose is introducing a nice developer interface ^ ^
#[derive(Event)]
pub enum ERC20Events {
   Transfer(from: Address, to: Address, value: U256),
   Approval(owner: Address, spender: Address, value: U256),
   // ...
}

#[zink::external]
fn foo(...) {
   ERC20Events::Transfer(from, to, value).emit()
}
If the described interface works, our PR works, we may encounter issues while implementing this interface, and they're the problems we need to solve in this PR, please pay attention to the inline document and the handbook as well since it will help others using our code!

ohh ok, will update soon, can i solve the log issue here also ?

clearloop · 2024-11-27T09:11:21Z

ohh ok, will update soon, can i solve the log issue here also ?

do you have an issue number of the log issue you mentioned? some of the issues may be outdated after our new implementation, feel free to add more Resolves #xx in the description of the PR!

on the other hand, we have limited budget issues, it would be proper that not solving two budget issues in one PR ^ ^

malik672 · 2024-12-03T07:40:34Z

@clearloop I might need your help here

clearloop

There will be two steps in this PR:

enum for static events

#[zink::Event]
pub enum MyEvent {
  MyStaticEvent1,
  MyStaticEvent2,
}

// which generates

pub struct MyEvent;

impl MyEvent {
  pub fn my_static_event_1() {
     ffi::evm::log0("MyStaticEvent1")
  }

  pub fn my_static_event_2() {
    ffi::evm::log0("My Static Event2")
  }
}

support dynamic event ( this is the main thing in this PR )

pub enum MyEvent {
   OneParam(Address), // log1
   TwoParams(Address, U256), // log2
   ThreeParams(Address, Address, U256), // log3
   FourParams(Address, Address, U256, U256) // log4
}

impl MyEvent {
  pub fn one_param(param1: Address) {
     // ... TODO HERE
  }

  pub fn two_params(param1: Address, param2: U256) {
    // ... TODO HERE
  }
  
  // ...
}

As we can see, ffi::evm::log0 only supports static bytes, which means it doesn't work for the dynamic events, so we need to refactor the log interfaces

//! zink/ffi/evm.rs
extern "C" {
  // ...

   // we keep this log0 because it works perfectly
   fn log0(name: &'static str);
   
   // the trick is here, we use `Bytes32` (each stack item is a bytes32 value in EVM) as the param instead of writing tons of interfaces for support all types
   fn log1(name: &'static str, topic1: Bytes32);
   
   // ...
}

so what we need to do now is introducing an interface that maps all types to Bytes32, in this PR, you just need to introduce the interfaces for Address and U256, for example for Address

zink/zink/src/primitives/address.rs

Lines 35 to 44 in e827947

    
           impl Asm for Address { 
        
               fn push(self) { 
        
                   unsafe { ffi::bytes::push_bytes20(self.0) } 
        
               } 
        
               #[cfg(not(target_family = "wasm"))] 
        
               fn bytes32(&self) -> [u8; 32] { 
        
                   self.0.bytes32() 
        
               } 
        
           }

impl Asm for Address {
  // ...
  
  #[cfg(target_family = "wasm")]
  fn to_bytes32(&self) -> Bytes32 {
    unsafe { ffi::asm::cast_bytes20(self.0) }
  }
}

implementing cast_bytesN in https://github.com/zink-lang/zink/blob/main/zink/src/primitives/bytes.rs#L37-L48 to support the logic above.

Update the log example tests, and this is the wrap!

malik672 · 2024-12-04T08:33:18Z

@clearloop somestuffs remain right ?

malik672 · 2024-12-04T13:23:53Z

@clearloop need some help

clearloop

Basically you're handling too many modules of zink at once )) it's complex tbh

see my comment on the log example, let's forget about the proc-macro and go with it first!

RUST_LOG=trace cargo nextest run --release --example log

you need to learn about debugging the compiled byteocde with the command above, paste the bytecode to the playground of evm.codes to verify if the generated bytecode are correct

clearloop · 2024-12-04T16:07:52Z

examples/log.rs

+pub enum ERC20Events {
+    /// Emitted when tokens are transferred between addresses
+    /// Parameters: from, to, value
+    Transfer(Address, Address, U256),
+
+    /// Emitted when an address approves another address to spend tokens
+    /// Parameters: owner, spender, value
+    Approval(Address, Address, U256),


Suggested change

pub enum ERC20Events {

/// Emitted when tokens are transferred between addresses

/// Parameters: from, to, value

Transfer(Address, Address, U256),

/// Emitted when an address approves another address to spend tokens

/// Parameters: owner, spender, value

Approval(Address, Address, U256),

pub enum MyEvent {

Topic0,

Topic1(U256),

Topic2(U256, U256),

Topic3(U256, U256, U256),

Topic4(U256, U256, U256, U256)

clearloop · 2024-12-04T16:08:27Z

examples/log.rs

+impl ERC20Events {
+    /// Log a transfer event
+    pub fn log_transfer(from: Address, to: Address, value: U256) {
+        match Self::Transfer(from, to, value) {
+            event => event.log0(),
+        }
+    }
+
+    /// Log an approval event
+    pub fn log_approval(owner: Address, spender: Address, value: U256) {
+        match Self::Approval(owner, spender, value) {
+            event => event.log0(),
+        }
+    }


Suggested change

impl ERC20Events {

/// Log a transfer event

pub fn log_transfer(from: Address, to: Address, value: U256) {

match Self::Transfer(from, to, value) {

event => event.log0(),

}

}

/// Log an approval event

pub fn log_approval(owner: Address, spender: Address, value: U256) {

match Self::Approval(owner, spender, value) {

event => event.log0(),

}

}

Since we have the proc-macro, don't need to implement them here

clearloop · 2024-12-04T16:11:02Z

examples/log.rs

+        let from = Address([1u8; 20]);
+        let to = Address([2u8; 20]);
+        let spender = Address([3u8; 20]);
+        let value = U256::from(1000u64);
+
+        // Test individual event logging
+        erc20_events::log_transfer(from, to, value)?;
+        erc20_events::log_approval(from, spender, value)?;

-#[zink::external]
-pub fn log4() {
-    Ping.log4(b"pong", b"ping", b"pong", b"pong");
+        // Test multiple events
+        erc20_events::log_transfer_and_approval(from, to, spender, value)?;


In the test, you need to call and external call and then in the returned info, check the logs to confirm they are correctly emitted, see the previous log example for template

clearloop · 2024-12-04T16:17:24Z

examples/log.rs

+    /// Logs a transfer event
+    /// Example contract functions demonstrating event logging
+    #[zink::external]
+    pub fn log_transfer(from: Address, to: Address, value: U256) {
+        ERC20Events::log_transfer(from, to, value)
+    }
+
+    /// Logs an approval event
+    /// Example contract functions demonstrating event logging
+    #[zink::external]
+    pub fn log_approval(owner: Address, spender: Address, value: U256) {
+        ERC20Events::log_approval(owner, spender, value)
+    }
+
+    /// Example of logging multiple events in a single transaction
+    /// Example contract functions demonstrating event logging
+    #[zink::external]
+    pub fn log_transfer_and_approval(
+        from: Address,
+        to: Address,
+        spender: Address,
+        value: U256,
+    ) -> Result<(), ()> {
+        // Log transfer first
+        log_transfer(from, to, value);
+        // Then log approval
+        log_approval(from, spender, value);
+        Ok(())
+    }


The testing interfaces are basically correct!

for now, let's temporarily forget about the proc-macro, the expanded logic of these methods should be

#[zink::external] pub fn topic0() { evm::ffi::log0("MyEvent"); } #[zink::external] pub fn topic1(value: U256) { ffi::evm::log1("MyEvent", value); } // ...

I'd suggest to complete this logic first, once it pass, generate them with proc-macro!

malik672 · 2024-12-05T12:52:39Z

@clearloop been looking at this code, still don't know why the macro complains of a U256 to bytes32 mismatch

clearloop · 2024-12-06T03:36:02Z

@clearloop been looking at this code, still don't know why the macro complains of a U256 to bytes32 mismatch

U256 is not equal to Bytes32,

pub struct U256(Bytes32)

mb you local branch forgot to merge the latest main? there was a big update for the bytes types xd

malik672 · 2024-12-06T04:54:37Z

@clearloop been looking at this code, still don't know why the macro complains of a U256 to bytes32 mismatch

U256 is not equal to Bytes32,
pub struct U256(Bytes32)
mb you local branch forgot to merge the latest main? there was a big update for the bytes types xd

ohh maybe that's why but i mean technically it's all the same number of bits

clearloop · 2024-12-06T05:01:00Z

@clearloop been looking at this code, still don't know why the macro complains of a U256 to bytes32 mismatch

U256 is not equal to Bytes32,
pub struct U256(Bytes32)
mb you local branch forgot to merge the latest main? there was a big update for the bytes types xd
ohh maybe that's why but i mean technically it's all the same number of bits

yes! exactly the same, however we need to introduce interfaces converting them when we need in rust to make rustc happy lmao

malik672 · 2024-12-06T05:07:46Z

@clearloop been looking at this code, still don't know why the macro complains of a U256 to bytes32 mismatch

U256 is not equal to Bytes32,
pub struct U256(Bytes32)
mb you local branch forgot to merge the latest main? there was a big update for the bytes types xd
ohh maybe that's why but i mean technically it's all the same number of bits
yes! exactly the same, however we need to introduce interfaces converting them when we need in rust to make rustc happy lmao

what do you think remains for this pr, implementation wise

clearloop

Your new interfaces of the example is correct! however on the other hand

you have modified the correct code to incorrect, the Address / Bytes conversion are correct in main
the tests of the logs are incorrect, you need to assert the output logs from EVM
once you can run log tests, you will reveal new error which requires some modifications in codegen, focus on 2. now

clearloop · 2024-12-06T05:22:40Z

examples/log.rs

+        // Test each log function
+        event_tests::test_log0();
+        event_tests::test_log1(value1);
+        event_tests::test_log2(value1, value2);
+        event_tests::test_log3(value1, value2, value3);
+        event_tests::test_log4(value1, value2, value3, value4);
+
+        // Test multiple logs
+        event_tests::test_multiple_logs(value1, value2, value3, value4).unwrap();


should compile the contract and then call these interfaces to complete the tests

@clearloop how do we assert the output, logs don't return yet

for example here:

https://github.com/zink-lang/zink/blob/main/examples/log.rs#L44-L50

we deploy the contract to our testing EVM

we call the method, in the returned info, there is a log field, see https://docs.zink-lang.org/rustdocs/zint/struct.Info.html#structfield.logs

clearloop · 2024-12-06T05:23:51Z