kkrt-labs / kakarot-ssj

Kakarot zkEVM - rewrite in the latest version of Cairo
https://www.kakarot.org
MIT License
137 stars 83 forks source link

dev: refactor `decode_legacy_tx` for consistency with typed transactions #979

Closed enitrat closed 1 month ago

enitrat commented 1 month ago

https://github.com/kkrt-labs/kakarot-ssj/blob/5352614daf538136ec421ef71a9ae95e64983d29/crates/utils/src/eth_transaction/transaction.cairo#L236

Refactor this function two explicitly split the two steps, as performed in decode_enveloped_typed_transaction

  1. The first step where, like in decode_enveloped_typed_transaction, we get rlp_decoded_data from decoding the encoded_tx_data and perform various checks on it
  2. The second step where we decode_fields to build the TxLegacy instance.

The decode_fields part must be moved to crates/utils/src/eth_transaction/legacy.cairo where, similarly to eip2930.cairo, we will implement a trait with the implementation of decode_fields

pub impl _impl of TxLegacyTrait {
    fn decode_fields(ref data: Span<RLPItem>) -> Result<TxLegacy, EthTransactionError> {
stevencartavia commented 1 month ago

🙋🏽‍♂️

martinvibes commented 1 month ago

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

hello i am a frontend dev and blockchain developer please can i work on this issue :) and would love to be a contributor

How I plan on tackling this issue

Here’s a summarized version of the refactor task for decode_legacy_tx:

Locate decode_legacy_tx in the transaction.cairo file. Split the function into two steps: Decode the rlp_decoded_data and perform checks. Move the field decoding into a separate function for building TxLegacy. Move the decode_fields logic to legacy.cairo and implement a decoding trait for TxLegacy. Update decode_legacy_tx to use the new trait and method. Test the changes to ensure proper functionality. Commit the refactor for review

ooochoche commented 1 month ago

@enitrat Please can I take on this instead

danielcdz commented 1 month ago

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

Hello, I'm Daniel Calderón! I'm a passionate software engineer with over 3 years of experience, actively contributing to open-source projects through OnlyDust. I’m also a community moderator at Dojo Coding, where I help new developers get started with blockchain development. Over the past few months, I've contributed to several Starknet projects, including cairo-vm-go by Nethermind. Currently, I’m a Cairo developer at ByteBuilders, working on ByteBeasts, a fully on-chain RPG game built using the Dojo Engine framework. My expertise spans full-stack development, with a focus on backend, frontend, and smart contract development in blockchain ecosystems.

How I plan on tackling this issue

ShantelPeters commented 1 month ago

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

i am blockchain developer with a strong backgound in cario. i am experienced in otheer languages like javascript, typescript html, please i will like to work on this issue as this would be my first time contributing to this ecosystem

How I plan on tackling this issue

To solve this issue,i will first refactor the function by splitting it into two steps. The first step involves getting rlp_decoded_data from decoding encoded_tx_data and performing checks. The second step should involve moving the decode_fields logic to crates/utils/src/eth_transaction/legacy.cairo. There, implement a trait similar to eip2930.cairo, with the decode_fields function inside the TxLegacyTrait to handle the decoding of TxLegacy.

enitrat commented 1 month ago

@ooochoche how are things going?

ooochoche commented 1 month ago

@enitrat I should be done within the next 48hrs

enitrat commented 1 month ago

Hi @ooochoche, it's been a week now - if you do not have any time to allocate to this, please let this be worked on by other contributors

ooochoche commented 1 month ago

I am creating a PR now