Skip to content
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: some fun unsafe stuff #412

Closed
wants to merge 28 commits into from

Conversation

Will-Smith11
Copy link
Contributor

@Autoparallel Remember at ethcc when we couldn't find a way around the PendingTransaction state and you had to end up forking ethers-rs...
Well I realized I had a fix and wanted to try this. I have a local test and the code works as expected... I don't care if you merge it or not I just wanted a low level challenge and thought best case scenario I can also help someone else out.

@Will-Smith11
Copy link
Contributor Author

https://github.com/Will-Smith11/mem-playground
if you want to test it btw

@Autoparallel
Copy link
Collaborator

@Will-Smith11 can we meet today and talk about this? I'd like to understand how it works. Dropping our ethers fork would be hugely beneficial.

I'll need one more feature than you list here, I believe. But nonetheless, this is awesome. I love to see it!!

@Will-Smith11 Will-Smith11 changed the base branch from feat/middleware to main August 16, 2023 23:24
Will-Smith11 and others added 4 commits August 16, 2023 20:39
Removed extraneous additions. Kept in the `PendingTxState` enum in `middleware.rs` for now.
@Autoparallel
Copy link
Collaborator

Just sent a PR into your branch @Will-Smith11

I reduced things a bit.

Let's keep it going and make sure we can get this to work with the least amount of overhead and with as much compatibility as possible.

@Autoparallel
Copy link
Collaborator

Your move, sir @Will-Smith11 😈

@0xJepsen
Copy link
Collaborator

0xJepsen commented Sep 7, 2023

Lets be sure to actually fill out all the useful fields in the transactions receipts

@codecov
Copy link

codecov bot commented Sep 7, 2023

Codecov Report

Merging #412 (7c813e3) into main (9c798a2) will increase coverage by 1.18%.
Report is 71 commits behind head on main.
The diff coverage is 74.00%.

❗ Current head 7c813e3 differs from pull request most recent head 09bc3eb. Consider uploading reports for the commit 09bc3eb to get more accurate results

@@            Coverage Diff             @@
##             main     #412      +/-   ##
==========================================
+ Coverage   58.24%   59.42%   +1.18%     
==========================================
  Files          10       11       +1     
  Lines        2996     3236     +240     
==========================================
+ Hits         1745     1923     +178     
- Misses       1251     1313      +62     
Files Changed Coverage Δ
bin/init.rs 0.00% <0.00%> (ø)
arbiter-core/src/bindings/block_info.rs 54.08% <54.08%> (ø)
arbiter-core/src/environment.rs 87.44% <87.12%> (+10.76%) ⬆️
arbiter-core/src/manager.rs 60.16% <90.00%> (-0.95%) ⬇️
arbiter-core/src/middleware.rs 89.25% <95.00%> (+4.73%) ⬆️
arbiter-core/src/math.rs 92.30% <100.00%> (+0.87%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

📢 Have feedback on the report? Share it here.

@0xJepsen
Copy link
Collaborator

0xJepsen commented Sep 7, 2023

I am going to close this PR and Re-open it into the feat(arbiter-core)/v0.5.0 branch as we will include this in the next minor version

@0xJepsen 0xJepsen closed this Sep 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: mutate PendingTransaction directly in memory to avoid custom ethers fork
3 participants