Fix/Morph client #2080
Fix/Morph client #2080
Conversation
carpawell
requested review from
alexvanin,
fyrchik and
cthulhu-rider
as code owners
Codecov Report
@@ Coverage Diff @@
## master #2080 +/- ##
==========================================
- Coverage 30.61% 30.60% -0.01%
==========================================
Files 380 380
Lines 28130 28139 +9
==========================================
Hits 8613 8613
- Misses 18778 18787 +9
Partials 739 739
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
I have recieved a notification of a new pull request. I am starting the build of images and binaries for further testing. Build number is 3682 |
|
I am running integration tests |
pkg/morph/client/notary.go
Outdated
| @@ -176,7 +175,7 @@ func (c *Client) DepositNotary(amount fixedn.Fixed8, delta uint32) (res util.Uin | |||
| big.NewInt(int64(amount)), | |||
| []interface{}{c.acc.PrivateKey().GetScriptHash(), till}) | |||
| if err != nil { | |||
| if !errors.Is(err, neorpc.ErrAlreadyExists) { | |||
| if !alreadyExistsError(err) { | |||
There was a problem hiding this comment.
Why didn't it work the old way?
There was a problem hiding this comment.
that is a server error, i did not find any code that returns exactly that err on the client side, did i miss anything?
There was a problem hiding this comment.
I think it doesn't matter here https://github.com/nspcc-dev/neo-go/blob/master/pkg/neorpc/errors.go#L131
There was a problem hiding this comment.
in fact, it seems like an unreachable code, cause all the TXs have their own nonce, i checked that
so ok, just dropped that commit
|
I have recieved a notification of a new pull request. I am starting the build of images and binaries for further testing. Build number is 3686 |
|
I am running integration tests |
|
Test run is finished. Please download the tarball from link. Untar and use |
cmd/neofs-node/morph.go
Outdated
| @@ -133,7 +133,7 @@ func makeNotaryDeposit(c *cfg) (util.Uint256, error) { | |||
|
|
|||
| // gasDivisor defines what part of GAS balance (1/gasDivisor) | |||
| // should be transferred to the notary service | |||
| gasDivisor = 2 | |||
| gasDivisor = 3 | |||
There was a problem hiding this comment.
What about all other, more frequent cases?
There was a problem hiding this comment.
i agree that it could not fix all the cases, i consider that as a hotfix: it will let a node survive two restarts in one block (+/- 8 sec). while that is a strange situation, we faced that in our deploys and are going to face it as a deployment rule for a while in the future
the true fix should try to analyze a current mempool. to be honest, i just do not know if it possible to do via RPC
There was a problem hiding this comment.
I don't think we need this hotfix in master -- it "fixes" a rather peculiar and artificial scenario.
Let's discuss and fix it properly in another task.
There was a problem hiding this comment.
Hm, 2 is still a magic value anyway cause nobody knows what exact value is perfect for a deposit but it certainly does not allow making two deposits (caused for any reason) in a row
dropped
|
I have recieved a notification of a new pull request. I am starting the build of images and binaries for further testing. Build number is 3689 |
|
I am running integration tests |
|
Test run is finished. Please download the tarball from link. Untar and use |
|
I have recieved a notification of a new pull request. I am starting the build of images and binaries for further testing. Build number is 3701 |
|
I am running integration tests |
|
I have recieved a notification of a new pull request. I am starting the build of images and binaries for further testing. Build number is 3704 |
|
I am running integration tests |
|
Test run is finished. Please download the tarball from link. Untar and use |
| @@ -38,6 +38,8 @@ Changelog for NeoFS Node | |||
| - Panic in IR when performing HEAD requests (#2069) | |||
| - Write-cache flush duplication (#2074) | |||
| - Ignore error if a transaction already exists in a morph client (#2075) | |||
| - Pack arguments of `setPrice` invocation during contract update (#2078) | |||
There was a problem hiding this comment.
rebase mistake, dropped
CHANGELOG.md
Outdated
| @@ -41,6 +41,7 @@ Changelog for NeoFS Node | |||
| - Pack arguments of `setPrice` invocation during contract update (#2078) | |||
| - Closing `neo-go` WS clients on shutdown and switch processes (#2080) | |||
| - Making notary deposits with a zero GAS balance (#2080) | |||
| - Notary requests on the shutdown (#2075) | |||
cmd/neofs-node/morph.go
Outdated
| @@ -133,7 +133,7 @@ func makeNotaryDeposit(c *cfg) (util.Uint256, error) { | |||
|
|
|||
| // gasDivisor defines what part of GAS balance (1/gasDivisor) | |||
| // should be transferred to the notary service | |||
| gasDivisor = 2 | |||
| gasDivisor = 3 | |||
There was a problem hiding this comment.
I don't think we need this hotfix in master -- it "fixes" a rather peculiar and artificial scenario.
Let's discuss and fix it properly in another task.
|
I have recieved a notification of a new pull request. I am starting the build of images and binaries for further testing. Build number is 3732 |
|
I am running integration tests |
|
I have recieved a notification of a new pull request. I am starting the build of images and binaries for further testing. Build number is 3733 |
|
I have recieved a notification of a new pull request. I am starting the build of images and binaries for further testing. Build number is 3734 |
|
I am running integration tests |
|
I have recieved a notification of a new pull request. I am starting the build of images and binaries for further testing. Build number is 3741 |
|
I am running integration tests |
|
Test run is finished. Please download the tarball from link. Untar and use |
Could be related to "websocket users limit reached" on the `neo-go` server side when an SN/IR is rebooting repeatedly. Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
|
I have recieved a notification of a new pull request. I am starting the build of images and binaries for further testing. Build number is 3752 |
|
I am running integration tests |
|
Test run is finished. Please download the tarball from link. Untar and use |
No description provided.