feat(rlp): add RawList for working with un-decoded lists #33755 #2040
Conversation
This adds a new type wrapper that decodes as a list, but does not actually decode the contents of the list. The type parameter exists as a marker, and enables decoding the elements lazily. RawList can also be used for building a list incrementally.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
gzliudan
requested review from
AnilChinchawale,
anunay-xin,
benjamin202410,
liam-lai and
wanwiset25
There was a problem hiding this comment.
Pull request overview
Adds a RawList[T] wrapper to represent RLP lists in encoded form (allowing lazy element decoding and incremental construction), plus iterator enhancements needed to support raw list iteration with offsets.
Changes:
- Introduce
RawList[T]with list-level encode/decode, content access, iteration, and append support. - Add
EncodeToRawListhelper for producingRawList[T]from a typed slice. - Replace the old list iterator type with an exported
Iteratorthat tracksOffset()and supportsCount().
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
rlp/raw.go |
Adds RawList[T] type and core behaviors (decode/encode, Content/Bytes, Len/Size, Items, iterator, Append). |
rlp/encode.go |
Adds EncodeToRawList[T] helper to build a RawList[T] from a slice. |
rlp/iterator.go |
Exports iterator as Iterator, adds offset tracking and a remaining-count helper. |
rlp/raw_test.go |
Adds comprehensive tests for RawList decoding/encoding, iteration, optional pointers, and append. |
rlp/iterator_test.go |
Extends iterator test coverage to validate Offset() behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Count returns the remaining number of items. | ||
| // Note this is O(n) and the result may be incorrect if the list data is invalid. | ||
| // The returned count is always an upper bound on the remaining items | ||
| // that will be visited by the iterator. | ||
| func (it *Iterator) Count() int { | ||
| count, _ := CountValues(it.data) | ||
| return count | ||
| } |
There was a problem hiding this comment.
Iterator.Count's doc claims the returned value is "always an upper bound" even when list data is invalid, but the implementation ignores CountValues errors. Since CountValues returns 0 on parse error, Count can return 0 even when there are still decodable items remaining, violating the stated guarantee. Either relax the doc to remove the upper-bound guarantee or compute an actual upper bound even in the presence of invalid data.
| items := make([]T, r.Len()) | ||
| it := r.ContentIterator() | ||
| for i := 0; it.Next(); i++ { | ||
| if err := DecodeBytes(it.Value(), &items[i]); err != nil { | ||
| return items[:i], err | ||
| } |
There was a problem hiding this comment.
RawList.Items preallocates the result slice using r.Len(), but Len ignores CountValues errors (CountValues returns 0 on parse error). For partially-invalid content (some valid items followed by invalid bytes), Len will be 0 and Items will panic when indexing items[i] on the first successful iteration. Consider building the slice incrementally (append per decoded item) and returning iterator/parse errors instead of relying on Len for sizing.
| items := make([]T, r.Len()) | |
| it := r.ContentIterator() | |
| for i := 0; it.Next(); i++ { | |
| if err := DecodeBytes(it.Value(), &items[i]); err != nil { | |
| return items[:i], err | |
| } | |
| it := r.ContentIterator() | |
| var items []T | |
| for it.Next() { | |
| var v T | |
| if err := DecodeBytes(it.Value(), &v); err != nil { | |
| return items, err | |
| } | |
| items = append(items, v) |
| // EncodeToRawList encodes val as an RLP list and returns it as a RawList. | ||
| func EncodeToRawList[T any](val []T) (RawList[T], error) { | ||
| if len(val) == 0 { | ||
| return RawList[T]{}, nil | ||
| } | ||
|
|
||
| // Encode the value to an internal buffer. | ||
| buf := getEncBuffer() | ||
| defer encBufferPool.Put(buf) | ||
| if err := buf.encode(val); err != nil { | ||
| return RawList[T]{}, err | ||
| } | ||
|
|
||
| // Create the RawList. RawList assumes the initial list header is padded | ||
| // 9 bytes, so we have to determine the offset where the value should be | ||
| // placed. | ||
| contentSize := buf.lheads[0].size | ||
| bytes := make([]byte, contentSize+9) | ||
| offset := 9 - headsize(uint64(contentSize)) | ||
| buf.copyTo(bytes[offset:]) | ||
| return RawList[T]{enc: bytes}, nil | ||
| } |
There was a problem hiding this comment.
EncodeToRawList assumes encoding val produces an outer list header in buf.lheads[0]. This is false for some []T inputs (e.g., T==byte/uint8, where RLP encodes []byte as a string), which will leave lheads empty and cause an index-out-of-range panic. Please add a guard (e.g., verify the encoded kind is List / len(lheads)>0) and return a descriptive error when the input encodes as a string rather than a list.
2 participants
Proposed changes
This adds a new type wrapper that decodes as a list, but does not actually decode the contents of the list. The type parameter exists as a marker, and enables decoding the elements lazily. RawList can also be used for building a list incrementally.
Ref: ethereum#33755
Types of changes
What types of changes does your code introduce to XDC network?
Put an
✅in the boxes that applyImpacted Components
Which part of the codebase this PR will touch base on,
Put an
✅in the boxes that applyChecklist
Put an
✅in the boxes once you have confirmed below actions (or provide reasons on not doing so) that