Skip to content

Conversation

@jsvisa
Copy link
Contributor

@jsvisa jsvisa commented Aug 26, 2023

fix #28008

@fjl
Copy link
Contributor

fjl commented Aug 26, 2023

It's nice you're submitting this, but I really think it should be handled in rlpgen instead. The method is there, so it should recognize that. The only reason why it does complain, is because function type-checking is turned on in the package loader.

@jsvisa
Copy link
Contributor Author

jsvisa commented Aug 26, 2023

Or we should use rlp.Encode instead of log.EncodeRLP(revert back) in

for _, log := range r.Logs {
if err := log.EncodeRLP(w); err != nil {
return err
}

just curious, why we changed from rlp.Encode to log.EncodeRLP in #27976

@fjl
Copy link
Contributor

fjl commented Aug 26, 2023

I changed it because it's way faster to call the method directly. The reason why rlpgen fails, is because it applies the build tag norlpgen during code generation.

@jsvisa
Copy link
Contributor Author

jsvisa commented Aug 26, 2023

@fjl thanks for the information, I changed the behavior in rlpgen, please take another look, thanks.

@fjl
Copy link
Contributor

fjl commented Sep 13, 2023

I decided to just always remove the build tag for now: #28106

@fjl fjl closed this Sep 13, 2023
@jsvisa jsvisa deleted the types-log-rlp branch September 14, 2023 06:14
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.

Go generate failed

2 participants