Implement RPC errors proposal #156

[Jim8y]

#728
#814

• [Plugins] Add neo submodule #873

[Jim8y]

@AnnaShaleva Please review

[shargon]

@Liaojinghui if you unify the namespace and fix the unit test we can merge it.

[Jim8y]

@Liaojinghui if you unify the namespace and fix the unit test we can merge it.

I need to talk with you about this pr since it changes the error code, do you think it is ok to merge it? This is inconsistent change.

[shargon]

@Liaojinghui if you unify the namespace and fix the unit test we can merge it.

I need to talk with you about this pr since it changes the error code, do you think it is ok to merge it? This is inconsistent change.

It will unify the error codes, I can't see the problem.

[Jim8y]

@Liaojinghui if you unify the namespace and fix the unit test we can merge it.

I need to talk with you about this pr since it changes the error code, do you think it is ok to merge it? This is inconsistent change.

It will unify the error codes, I can't see the problem.

Fair enough, I will work on it

[Jim8y]

@roman-khimov will need you to define some new error code for the canceltx command. The existing error code are like randomly assigned. As to the format, i run dotnet format everytime i commit.

[AnnaShaleva]

@roman-khimov will need you to define some new error code for the canceltx command.

I don't think we need a separate error code. canceltx creates a simple transaction with Conflicts attribute, it uses a standard transaction relay mechanism, and the error codes should be the same as for the sendrawtransaction. When it comes to NeoGo implementation, we have a bit different CLI scheme and we'll use the standard sendrawtransaction RPC call and send the conflicting transaction from NeoGo CLI to the provided RPC node endpoint (it will be done within the scope of nspcc-dev/neo-go#3151). So I'd say that the error codes will be the same as described at https://github.com/neo-project/proposals/pull/156/files#diff-2b5f7c12a23f7dbe4cb46bbf4be6936882f8e0f0b3a4db9d8c58eb294b02e6edR288-R289.

@Liaojinghui, does this comment answer your question?

[Jim8y]

@superboyiii good to go for testing.

[shargon]

We will need this PR in the mono repo, we don't have nuget, so, you can't update neo (for AlreadyInPool).

[Jim8y]

We will need this PR in the mono repo, we don't have nuget, so, you can't update neo (for AlreadyInPool).

Can use local reference to test. Postpone the monorepo merge can make things unnecessarily complex. Waiting for a release is nonsense.

[roman-khimov]

We need this.

[shargon]

Looks good to me if neo-go team are good with it

[roman-khimov]

We actually have spotted some minor problems in the set of errors passed from core to module (we can have a bit more details there), but I don't think these should hold the PR. We can also improve the proposal now with Conflicts handling that was missing from the protocol when it was initially submitted, but that's also a different topic. The PR here is a pretty big and good set of changes, it definitely improves the situation and it's compliant to the current state of proposal, so let's merge it as is and then improve upon it if and when needed.