coderabbitai[bot]
commented
1 month ago [!CAUTION]
Review failed
The pull request is closed.
Walkthrough
The recent changes enhance the blockchain API's log retrieval functionality by improving type handling and ensuring more robust error management. Key modifications include transitioning from big.Int to uint64 for block height parameters, streamlining the codebase for better performance and clarity. Additionally, the handling of response structures ensures consistent outputs, reducing the risk of errors when retrieving logs, especially for early block numbers.
Changes
| Files | Change Summary |
|---|---|
api/api.go, api/pull.go |
Updated GetLogs and getLogs methods to improve type handling for block heights, transitioning from big.Int to uint64, simplifying logic and enhancing performance. |
models/receipt.go |
Adjusted BloomsHeight struct to use uint64 instead of *big.Int for height, improving memory efficiency and performance. |
services/logs/filter.go, filter_test.go |
Changed RangeFilter to utilize uint64 for start and end, improving filtering logic and performance. Adjusted tests accordingly to reflect new parameter types. |
storage/index.go, index_testsuite.go |
Updated BlockIndexer and ReceiptIndexer methods to replace *big.Int with uint64, enhancing clarity and consistency in method signatures. |
storage/mocks/*.go |
Modified mocks to align with changes in parameter types from *big.Int to uint64, ensuring consistency across the mocked functions. |
storage/pebble/*.go |
Transitioned from using big.Int to uint64 for height-related operations, streamlining code and improving performance across multiple methods. |
Assessment against linked issues
| Objective | Addressed | Explanation |
|---|---|---|
| Return only the logs from the specified range in the results (#[439]) | ✅ |
🐰🌟 In the blockchain glade, where transactions hop,
Types simplified, and errors drop.
From big integers, we've made a flight,
Nowuint64shines, so clear and bright!
Logs will filter as they should, oh what a delight! 🌟🐰
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?
Closes: #439
Description
The storage implementation for bloom filters incorrectly defined the range which resulted in additional logs being returned.
Because we've used
big.Inttype for heights, more specificallybig.Int.Bytes()which returned byte array that only contained non-empty values in big-endian encoding, it allowed for multiple other values to match the pebble itteration upper/lower bound filter.Simply said, if we wanted to get height 1 when converted using
Bytes()we got a slice[]byte{0x1}and that was used as boundary in iterations, but that also matched other values starting with same byte (like[]byte{0x01, 0x02}or[]byte{0x01, 0xf, 0xa}). We changed usage ofbig.Inttouint64since it doesn't really make sense in storage layer to usebig.Int, and we changed to encoding of uint64 to fixed length of 8 bytes. So the above example of height 1 would be defined as[]byte{0x1, 0x0, 0x0, 0x0}thus no other value would match it.For contributor use:
masterbranchFiles changedin the Github PR explorerSummary by CodeRabbit
New Features
*big.Inttouint64).Bug Fixes
nilreference errors.Documentation
Tests