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

In fork.py's make_receipt, multiple if/return blocks are followed by an else #874

Closed
SamWilsn opened this issue Feb 1, 2024 · 3 comments
Assignees
Labels
A-spec Area: specification C-enhance Category: a request for an improvement E-easy Experience: easy, good for newcomers

Comments

@SamWilsn
Copy link
Contributor

SamWilsn commented Feb 1, 2024

For example:

if isinstance(tx, AccessListTransaction):
return b"\x01" + rlp.encode(receipt)
if isinstance(tx, FeeMarketTransaction):
return b"\x02" + rlp.encode(receipt)
else:
return receipt

This is a really weird construct. Either they should all be elif or we shouldn't have the else:.

Originally posted by @SamWilsn in #867 (comment)

@SamWilsn SamWilsn changed the title In fork.py, multiple if/return blocks are followed by an else In fork.py's make_receipt, multiple if/return blocks are followed by an else Feb 1, 2024
@SamWilsn SamWilsn added this to the Post-Cancun Refactoring milestone Feb 1, 2024
@SamWilsn SamWilsn added A-spec Area: specification C-enhance Category: a request for an improvement E-easy Experience: easy, good for newcomers labels Feb 1, 2024
@richardgreg
Copy link
Contributor

Could you assign me to this task?

@SamWilsn SamWilsn assigned SamWilsn and richardgreg and unassigned SamWilsn Feb 5, 2024
@SamWilsn
Copy link
Contributor Author

SamWilsn commented Feb 5, 2024

@richardgreg feel free to take a stab at it (don't forget to make a separate PR targeting forks/cancun). Thanks!

@richardgreg
Copy link
Contributor

richardgreg commented Feb 9, 2024

(don't forget to make a separate PR targeting forks/cancun). Thanks!

Because of the above comment, I opened a PR targeting the cancun branch. lol. I hope I did the right thing. If not, let me know and I'll open it against the master branch 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-spec Area: specification C-enhance Category: a request for an improvement E-easy Experience: easy, good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants