Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RpcServer: return RpcError.InsufficientNetworkFee error where appropriate #889

Open
AnnaShaleva opened this issue Mar 18, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@AnnaShaleva
Copy link
Member

Describe the bug
After #815 merge, RpcError.InsufficientFunds (with code -511) is returned on the incoming transaction verification when:

  1. Sender's balance is not enough to pay for all his transactions in the pool
  2. Sender's balance is not enough to pay for the particular incoming transaction's fees

Whereas RPC errors proposal postulates that -511 Insufficient funds error should be returned only in case 1, i.e. when:

| -511 || Insufficient funds || Sender doesn't have enough GAS to pay for all currently pooled transactions.

And all other cases of insufficient sender's balance should be covered by generic -500 error:

| -500 || Unclassified verification error|| Anything that can't be expressed by other codes.

To Reproduce
Steps to reproduce the behavior:

  1. Check the transaction verification code from core: https://github.com/neo-project/neo/blob/b05501af882a0d1f2a1a7841c6ddc4d0504e5fc1/src/Neo/Network/P2P/Payloads/Transaction.cs#L368-L395
  2. Check the error conversion code on the RPC server side:
    case VerifyResult.InsufficientFunds:
    {
    throw new RpcException(RpcError.InsufficientFunds.WithData(reason.ToString()));
    }

Expected behavior

  1. We need to modify TransactionVerificationContext.CheckTransaction so that it returns some new special error, this error should be detached from VerifyResult.InsufficientFunds and ported to other components (ref. https://github.com/neo-project/neo/blob/b05501af882a0d1f2a1a7841c6ddc4d0504e5fc1/src/Neo/Network/P2P/Payloads/Transaction.cs#L368.). This special error should be converted to RpcError with -511 code on the RPC server side whereas the rest of VerifyResult.InsufficientFunds cases should be converted to RpcError with -500 code.
  2. We may need to adjust error text of RpcError.ExpiredTransaction to make it more precise to reflect the proposal intention. Currently it's:
    public static readonly RpcError InsufficientFunds = new(-511, "Insufficient funds for fee");

Platform:

  • Version: RpcServer, current master (253a77e).

Additional context
The same problem was recently fixed in NeoGo, see nspcc-dev/neo-go#3360. This issue is not critical, but eventually it should be fixed.

@roman-khimov
Copy link
Contributor

roman-khimov commented Mar 18, 2024

That's not the way it was intended to be, sorry. See nspcc-dev/neo-go#3361. The problem with C# code/modules is that it never returns -504.

I'll improve neo-project/proposals#156 wording about -511 though.

@AnnaShaleva
Copy link
Member Author

Good, then no changes from neo-modules or core side are required, thus I'm closing this issue as not planned.

@AnnaShaleva AnnaShaleva closed this as not planned Won't fix, can't repro, duplicate, stale Mar 18, 2024
@AnnaShaleva
Copy link
Member Author

The problem with C# code/modules is that it never returns -504.

Although we may use this issue to enable -504 in neo-modules (and core).

@AnnaShaleva AnnaShaleva reopened this Mar 18, 2024
@AnnaShaleva AnnaShaleva changed the title Adjust the usages of RpcError.InsufficientFunds and RpcError.VerificationFailed errors RpcServer: return RpcError.InsufficientNetworkFee error where appropriate Mar 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants