RLP encoding and decoding generator for simple structures #13341
Conversation
core/types/encdec_test.go
Outdated
| } | ||
|
|
||
| func TestTestingStruct(t *testing.T) { | ||
| // tr := NewTRand() |
There was a problem hiding this comment.
Currently there are no EncodeRLP and DecodeRLP methods for TestingStruct, so the program wont compile due to the errors. Methods have to be generated and then this can be uncommented and tested. But in general, I suggest not to merge this to the main, since this required only for generator testing. Let me make the "merge-ready" version after I demonstrate and explain how does this work in the next upcoming call
| _ = addToImports(named) | ||
| } | ||
| // size | ||
| fmt.Fprintf(b1, " size += rlp.BigIntLenExcludingHead(&obj.%s) + 1\n", fieldName) |
There was a problem hiding this comment.
+1 only works if the number of significant bytes in the bigInt is smaller than 56, which theoretically might be not the case, though practically our bigInts are no longer than 32 bytes.
There was a problem hiding this comment.
I suggest adding extra check in BigIntLenExcludingHead function
if bitLen >= 56*8 { return libcommon.BitLenToByteLen(bitLen) - 1 } // so the caller can add +1 without hesitationThere was a problem hiding this comment.
That's actually not going to work
There was a problem hiding this comment.
OK, it's not a big deal and we can leave this as is since our bigInts are no longer than 32 bytes in practice.
go.mod
Outdated
| @@ -282,7 +282,7 @@ require ( | |||
| go.uber.org/multierr v1.11.0 // indirect | |||
| golang.org/x/mod v0.21.0 // indirect | |||
| golang.org/x/text v0.21.0 // indirect | |||
| golang.org/x/tools v0.26.0 // indirect | |||
| golang.org/x/tools v0.26.0 | |||
erigon-lib/rlp/encode.go
Outdated
| return nil | ||
| } | ||
|
|
||
| func ByteSliceSlicePtrSize(bb []*[]byte) int { |
There was a problem hiding this comment.
Why do we need this? To my mind it doesn't make sense to have pointer to a slice since slices have reference semantics.
There was a problem hiding this comment.
Agree, should be removed
cmd/rlpgen/handlers.go
Outdated
| @@ -0,0 +1,734 @@ | |||
| // Copyright 2023 The Erigon Authors | |||
|
Seems like I don't have a rights for |
The manifest check is flaky and unrelated to your changes, so can be ignored. |
No description provided.