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

rlp: avoid copy in encode #2476

Merged
merged 1 commit into from
Jan 10, 2023

Conversation

wemeetagain
Copy link
Contributor

Avoid an intermediate copy when encode is called with an Array.

@codecov
Copy link

codecov bot commented Jan 9, 2023

Codecov Report

Merging #2476 (ebf54d8) into master (f917530) will increase coverage by 1.20%.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

Flag Coverage Δ
block ?
blockchain 90.04% <ø> (ø)
client 87.82% <ø> (ø)
common 95.89% <ø> (ø)
devp2p 91.62% <ø> (-0.06%) ⬇️
ethash ∅ <ø> (∅)
evm ?
rlp ∅ <ø> (?)
statemanager 89.61% <ø> (ø)
trie 90.70% <ø> (+0.68%) ⬆️
tx ?
util 84.74% <ø> (ø)
vm 85.65% <ø> (ø)

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

@wemeetagain wemeetagain changed the title chore: avoid copy in rlp.encode rlp: avoid copy in encode Jan 10, 2023
Copy link
Contributor

@acolytec3 acolytec3 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the catch!

@acolytec3 acolytec3 merged commit 70dbdd1 into ethereumjs:master Jan 10, 2023
@gitpoap-bot
Copy link

gitpoap-bot bot commented Jan 10, 2023

Congrats, your important contribution to this open-source project has earned you a GitPOAP!

GitPOAP: 2023 EthereumJS Contributor:

GitPOAP: 2023 EthereumJS Contributor GitPOAP Badge

Head to gitpoap.io & connect your GitHub account to mint!

Learn more about GitPOAPs here.

@wemeetagain
Copy link
Contributor Author

Curious when this would land in a release?

@acolytec3
Copy link
Contributor

Curious when this would land in a release?

Unfortunately, hard to say exactly. Might be able to get a point release out in a couple of weeks but it depends on what other PRs we've merged that might need to get included and what all of our downstream packages also have to be released. Is this urgent for lodestar?

@wemeetagain
Copy link
Contributor Author

not urgent at all

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants