cli, blockfetcher, statefetcher: migrate to searchv2 #3913
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| return 0, fmt.Errorf("no state object found") | ||
| } | ||
| s.lock.Lock() | ||
| s.lastStateObjectIndex = uint32(lastFoundIdx) |
Check failure
Code scanning / CodeQL
Incorrect conversion between integer types High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 days ago
To fix the issue, we need to ensure that the parsed integer (lastFoundIdx) is within the valid range for uint32 before performing the conversion. This can be achieved by adding explicit bounds checks using constants from the math package (math.MaxUint32 for the upper bound and 0 for the lower bound). If the value is out of range, the function should handle the error gracefully, such as by returning an error or a default value.
The changes will be made in the LatestStateObjectHeight function in the file pkg/services/statefetcher/statefetcher.go. Specifically:
- Add bounds checks for
lastFoundIdxafter parsing it withstrconv.Atoi. - If the value is out of range, return an appropriate error message.
| @@ -170,2 +170,7 @@ | ||
| } | ||
| // Check bounds for uint32 | ||
| if lastFoundIdx < 0 || lastFoundIdx > math.MaxUint32 { | ||
| s.isActive.CompareAndSwap(true, false) | ||
| return 0, fmt.Errorf("state object index out of range for uint32: %d", lastFoundIdx) | ||
| } | ||
| if lastFoundIdx == 0 || items[len(items)-1].ID.IsZero() { |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3913 +/- ##
==========================================
+ Coverage 81.78% 81.88% +0.09%
==========================================
Files 345 345
Lines 48852 48809 -43
==========================================
+ Hits 39954 39967 +13
+ Misses 7229 7177 -52
+ Partials 1669 1665 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Rebase, please. |
No need of additional PoolWrapper and loosing pool errors. Signed-off-by: Ekaterina Pavlova <[email protected]>
Reverting c955c1e as nspcc-dev/neofs-node#2988 is fixed. Close #3656 Signed-off-by: Ekaterina Pavlova <[email protected]>
Close #3825 Signed-off-by: Ekaterina Pavlova <[email protected]>
Revert #3670. As NeoFS searchv2 result limit is 1000, the DefaultSearchBatchSize could be little bit less to get full range with one request, even if there are duplicates. Close #3707 Signed-off-by: Ekaterina Pavlova <[email protected]>
AliceInHunterland
force-pushed
the
neofs-sdk
branch
from
11fee29 to
eb3ada1
Compare
AliceInHunterland
requested review from
AnnaShaleva and
roman-khimov
as code owners
There was a problem hiding this comment.
Pull Request Overview
This PR migrates several NeoFS-related modules—cli, blockfetcher, and statefetcher—to use the new SearchV2 API to address multiple issues. Key changes include refactoring the search and retrieval logic in statefetcher and blockfetcher, updating pool handling in multiple modules, and adjusting the index file and state upload workflows.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/services/statefetcher/statefetcher.go | Updated LatestStateObjectHeight to use SearchV2 filters and type conversion. |
| pkg/services/helpers/neofs/neofs.go | Changed the ObjectSearch API signature and adjusted error handling. |
| pkg/services/helpers/neofs/blockstorage.go | Updated DefaultSearchBatchSize and changed PoolWrapper usage. |
| pkg/services/blockfetcher/blockfetcher.go | Migrated object search and retrieval logic, using object IDs from SearchV2. |
| cli/util/upload_state.go | Revised state object search logic with updated loop boundaries. |
| cli/util/upload_bin.go | Updated pool usage and adjusted index file search and block upload logic. |
| cli/options/options.go | Modified pool type handling in GetNeoFSClientPool. |
Comments suppressed due to low confidence (3)
pkg/services/blockfetcher/blockfetcher.go:386
- [nitpick] The condition 'if startIndex == startIndex+batchSize-1' is confusing; consider explicitly checking if batchSize equals 1 for improved readability.
if startIndex == startIndex+batchSize-1 {
pkg/services/helpers/neofs/blockstorage.go:32
- [nitpick] Ensure that changing DefaultSearchBatchSize from 1 to 950 is based on performance benchmarks and that it does not inadvertently introduce duplicate object issues.
DefaultSearchBatchSize = 950
cli/util/upload_state.go:107
- [nitpick] Verify that the adjustment of the starting index for uploading states (from stateObjCount to stateObjCount + 1) aligns with the intended off-by-one logic in the state upload process.
for state := stateObjCount + 1; state <= currentStateIndex; state++ {
| @@ -126,6 +126,7 @@ func New(chain Ledger, cfg config.NeoFSStateFetcher, stateSyncInterval int, logg | |||
| return s, nil | |||
| } | |||
|
|
|||
| // LatestStateObjectHeight returns the latest found in the container state object height. | |||
There was a problem hiding this comment.
[nitpick] Consider rephrasing the doc comment for clarity. For example, "LatestStateObjectHeight returns the height of the most recent state object found in the container."
Copilot uses AI. Check for mistakes.
Signed-off-by: Ekaterina Pavlova <[email protected]>
AliceInHunterland
force-pushed
the
neofs-sdk
branch
from
eb3ada1 to
78f762c
Compare
Close #3656
Close #3825
Close #3707