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(primitives): effective_gas_tip and effective_tip_per_gas functions together #5144

Merged
merged 12 commits into from
Nov 2, 2023

Conversation

DoTheBestToGetTheBest
Copy link
Contributor

Should close this #5141

@codecov
Copy link

codecov bot commented Oct 23, 2023

Codecov Report

Merging #5144 (f0163ea) into main (36dde36) will decrease coverage by 55.05%.
Report is 77 commits behind head on main.
The diff coverage is 40.00%.

❗ Current head f0163ea differs from pull request most recent head 2569e83. Consider uploading reports for the commit 2569e83 to get more accurate results

Impacted file tree graph

Files Coverage Δ
crates/transaction-pool/src/traits.rs 35.16% <100.00%> (-8.43%) ⬇️
crates/primitives/src/transaction/mod.rs 25.13% <87.50%> (-54.00%) ⬇️
crates/rpc/rpc/src/eth/api/fees.rs 0.00% <0.00%> (-49.24%) ⬇️
crates/rpc/rpc/src/eth/bundle.rs 0.00% <0.00%> (ø)
crates/payload/basic/src/lib.rs 0.00% <0.00%> (ø)
crates/rpc/rpc-types-compat/src/transaction/mod.rs 0.00% <0.00%> (ø)
crates/rpc/rpc/src/eth/gas_oracle.rs 20.45% <0.00%> (-19.32%) ⬇️

... and 449 files with indirect coverage changes

Flag Coverage Δ
integration-tests 12.94% <40.00%> (-4.12%) ⬇️
unit-tests ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
reth binary 6.18% <ø> (-24.40%) ⬇️
blockchain tree 0.00% <ø> (-80.83%) ⬇️
pipeline 0.00% <ø> (-88.37%) ⬇️
storage (db) 15.20% <ø> (-59.38%) ⬇️
trie 0.00% <ø> (-94.97%) ⬇️
txpool 25.75% <100.00%> (-30.02%) ⬇️
networking 26.47% <ø> (-51.89%) ⬇️
rpc 10.65% <0.00%> (-47.05%) ⬇️
consensus 0.00% <ø> (-63.02%) ⬇️
revm 0.49% <ø> (-23.99%) ⬇️
payload builder 0.00% <0.00%> (-7.96%) ⬇️
primitives 15.28% <87.50%> (-69.50%) ⬇️

@mattsse
Copy link
Collaborator

mattsse commented Oct 25, 2023

ptal @Rjected

Copy link
Member

@Rjected Rjected left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good start, just have some small changes so far

let base_fee = base_fee as u128;
pub fn effective_tip_per_gas(&self, base_fee: Option<u64>) -> Option<u128> {
// Convert the base fee to u128 if present, or provide a default value
let base_fee = base_fee.map_or(0, |fee| fee as u128);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should use unwrap_or_default here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should use unwrap_or_default here

Hello, thank you for these information. Deleted comments + used unwrap_or_default :)

crates/primitives/src/transaction/mod.rs Outdated Show resolved Hide resolved
Copy link
Member

@Rjected Rjected left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just one change that should have been caught earlier, then this looks good

crates/primitives/src/transaction/mod.rs Outdated Show resolved Hide resolved
@DoTheBestToGetTheBest
Copy link
Contributor Author

just one change that should have been caught earlier, then this looks good

Hey, thank you for this. I changed the missing part :)

Copy link
Member

@Rjected Rjected left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

smol style nit

crates/primitives/src/transaction/mod.rs Outdated Show resolved Hide resolved
Copy link
Member

@Rjected Rjected left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks good to me now

crates/primitives/src/transaction/mod.rs Outdated Show resolved Hide resolved
crates/primitives/src/transaction/mod.rs Outdated Show resolved Hide resolved
@mattsse mattsse added this pull request to the merge queue Nov 2, 2023
Merged via the queue into paradigmxyz:main with commit 93fc2e2 Nov 2, 2023
22 checks passed
mattsse added a commit that referenced this pull request Nov 8, 2023
…ns together (#5144)

Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>
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.

3 participants