kkrt-labs / kakarot-rpc

Kakarot ZK EVM Ethereum RPC adapter
MIT License
136 stars 101 forks source link

dev: simplify `WithOtherFields<T>` usage #1398

Closed tcoratger closed 3 weeks ago

tcoratger commented 1 month ago

In our codebase, we use the other field of transactions to store isRunOutOfResources information. This leads to extensive usage of WithOtherFields<T> throughout the code after the merge of https://github.com/kkrt-labs/kakarot-rpc/pull/1389, especially when handling conversions between RPC and primitive types, as seen in https://github.com/paradigmxyz/reth/blob/fba837468cac5143b415c9dd4fdc9218d71a926b/crates/primitives/src/alloy_compat.rs#L55-L212.

However, this approach is complex and can be optimized for better readability and management.

Proposal

  1. Simplify where possible: identify and remove unnecessary uses of WithOtherFields<T>, replacing them with just T where the extra fields aren't needed.
  2. Define a reusable type: introduce a type alias such as ExtendedTransaction = WithOtherFields<Transaction> (and similar for blocks) to standardize its usage where necessary.
Samrath49 commented 1 month ago

Hi @tcoratger, I would like to contribute to this issue, kindly assign me.

onlydustapp[bot] commented 1 month ago

Hey @Samrath49! Thanks for showing interest. We've created an application for you to contribute to Kakarot zkEVM. Go check it out on OnlyDust!

tcoratger commented 1 month ago

@Samrath49 Assigned

greged93 commented 1 month ago

@Samrath49 any update on this?

maxslimb commented 1 month ago

@tcoratger can you explain more about the issue with references to simplifying WithOtherFields usages?

sajalbnl commented 1 month ago

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

I want to work on this

udyan13 commented 1 month ago

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

As a seasoned Rust developer, I have a proven track record of optimizing complex codebases for both performance and readability. My experience includes refactoring legacy code, implementing type aliases, and working with RPC systems. I'm confident in my ability to improve the clarity and maintainability of our WithOtherFields code while ensuring functionality remains intact. I'm a collaborative problem-solver who is committed to producing high-quality code.

How I plan on tackling this issue

The summary you provided is clear and well-structured. Here are a few points to consider for improvement and clarity:

Strengths Clarity: The steps are articulated in a logical order, making it easy to follow your approach. Comprehensive: It covers all essential aspects, from analysis to documentation and communication. Focus on Collaboration: Emphasizing team communication is a great addition, as it highlights a collaborative mindset. Minor Suggestions Conciseness: While the paragraph is clear, you might condense some phrases to enhance readability. Terminology Consistency: Ensure terms like "redundant" and "essential" are clearly defined if the audience may not be familiar with them. Revised Version Here’s a slightly refined version:

To approach the problem, I would start with a thorough code review to identify all instances of WithOtherFields and assess their necessity. These usages would be categorized into three groups: essential cases required for functionality, redundant cases that can be safely removed, and ambiguous cases needing further team discussion. For the redundant cases, I would develop a plan to replace WithOtherFields with T, documenting the rationale for retaining it in essential cases. Additionally, I would introduce type aliases, such as ExtendedTransaction and ExtendedBlock, to standardize usage. Ensuring comprehensive testing is crucial; I would run existing tests and add new ones following the changes. I would also update relevant documentation to reflect these modifications and provide guidelines for future usage. Throughout the process, maintaining open communication with the team would be essential for gathering feedback and ensuring alignment. This structured approach aims to reduce complexity while preserving functionality and enhancing code readability.

chigozzdevv commented 1 month ago

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

Hey there! I'm really excited about this task - it's right up my alley. I've been working with Rust and blockchain tech for a while now, and I love digging into codebases to make them cleaner and more efficient.

How I plan on tackling this issue

For this problem, here's how I'd tackle it: First off, I'd dive into the code and really get a feel for how we're using WithOtherFields. It's always good to understand the full picture before making changes. Then, I'd go through and look for any easy wins - places where we can just use T instead of WithOtherFields without losing anything important. It's surprising how often we can simplify things with a fresh pair of eyes. I like the idea of creating those type aliases. It'd make the code much easier to read and understand. I'd probably call them something like ExtendedTransaction and ExtendedBlock, and make sure to add some clear comments about when to use them. When it comes to making changes, I prefer to take it step by step. Start with the obvious stuff, test it out, then move on to the trickier parts. It helps catch any issues early on.

PabloVillaplana commented 1 month ago

Hey Thomas @tcoratger Can I take this one? ☝️

MooreTheAnalyst commented 3 weeks ago

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

I am a data scientist who is skilled in python, data analysis, machine learning, and problem solving. My skills can be leveraged for problem-solving and code manipulation

How I plan on tackling this issue

1.Remove unnecessary WithOtherFields: Replace it with T where possible. 2.Create type alias: Use ExtendedTransaction for WithOtherFields. 3.Apply alias consistently: Standardize its usage.

Goodnessukaigwe commented 3 weeks ago

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

As a Python-proficient data scientist, i excel in cleaning, exploring, and visualizing data, as well as building models to solve real-world business problems. My strengths in data wrangling, exploratory data analysis, and clear communication of insights make me a valuable asset for driving data-driven decision-making.

How I plan on tackling this issue

Fristly, i will conduct a thorough review of your codebase to find instances where WithOtherFields is used but not needed.For example, i look for places where isRunOutOfResources is not being accessed or used. Then, Change instances of WithOtherFields to just T where applicable. After that i will create a type aliases to standardize the usage of extended fields. Finally, replace all relevant instances with the new type aliases.

bruhhgnik commented 3 weeks ago

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

I am a core linux dev, i have worked on ci/cd pipelines before, alongside that i have worked on ML models, i am on an outlook to learn more in more fields, i think ill be able to tackle this issue

How I plan on tackling this issue

a) Audit current usage of WithOtherFields b) Createe type aliases (ExtendedTransaction, etc.) c) Refactor code to use new types where appropriate d) Update conversion functions between RPC and primitive types e) Thoroughly test changes f) Update documentation

Gerson2102 commented 3 weeks ago

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

Hey! I'm Gerson. 👋

Member of Dojo Coding. I have contributed before to other Kakarot projects like:

Can I work on this issue?

Check my OnlyDust profile, I've contributed to many projects: Profile


How I plan on tackling this issue

Plan to Solve the Issue:

  1. Study the codebase to understand the current structure and functionality.
  2. Understand the problem that needs to be solved by thoroughly reading the issue details.
  3. Leverage all provided resources within the issue to get a better grasp of the task at hand.
  4. If I encounter roadblocks, I will ask questions ASAP in the Telegram group to move forward effectively.
  5. I will open a PR and wait for ur feedback

tcoratger commented 3 weeks ago

@Gerson2102 assigned

Gerson2102 commented 3 weeks ago

Hey @tcoratger. So where should I look for all the usages of the WithOtherFields<T> inside of the code? Maybe all those files that u changed on the linked PR of the issue description?