search

nspcc-dev / neofs-node

NeoFS is a decentralized distributed object storage integrated with the Neo blockchain
https://fs.neo.org
GNU General Public License v3.0
31 stars 38 forks source link

fix: Do not allow broken session token by SN #2727

Closed carpawell closed 7 months ago

carpawell commented 7 months ago

If container session token is incorrect, there is no need in Alphabet's check, the error can be returned immediately to a client; otherwise he has to wait for the TX to persist infinitely with not feedback. Closes #2466.

carpawell commented 7 months ago

with these changes, broken sessions' verification by the IR wont be tested anymore since SNs wont bypass such sessions, right?

Seems so. But that is how the issues like "do not want to wait too long before IR reacts, pls, make it on the SN side" are fixed. Do not see any problems here.

IR tests are critical, so we must not lose them

Can be checked with neo-go and notary requests anyway.

codecov[bot] commented 7 months ago

Codecov Report

Attention: 10 lines in your changes are missing coverage. Please review.

Comparison is base (895559b) 28.81% compared to head (53dc640) 28.84%.

:exclamation: Current head 53dc640 differs from pull request most recent head f8620ff. Consider uploading reports for the commit f8620ff to get more accurate results

Files Patch % Lines
pkg/services/container/morph/executor.go 66.66% 5 Missing and 5 partials :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #2727 +/- ## ========================================== + Coverage 28.81% 28.84% +0.03% ========================================== Files 415 415 Lines 32397 32427 +30 ========================================== + Hits 9335 9355 +20 - Misses 22228 22233 +5 - Partials 834 839 +5 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

cthulhu-rider commented 7 months ago

Can be checked with neo-go and notary requests anyway.

not everybody will be ready for this. I'd add a remark to Updating section of the changelog

carpawell commented 7 months ago

I'd add a remark to Updating section of the changelog

Not sure what an updater should see in this line. Not sure how @532910 will react. But I tried a little.

cthulhu-rider commented 7 months ago

But I tried a little.

anything i can look at?

carpawell commented 7 months ago

anything i can look at?

Oh, has not pushed, my bad.