From fe6c7cef9813ebf39807f5e353e4479bfe6e951c Mon Sep 17 00:00:00 2001 From: kaldun-tech Date: Wed, 29 Apr 2026 11:58:52 -0600 Subject: [PATCH 1/2] rpcserver: return InvalidArgument for RPC validation errors Update DecodeAddr, DecodeProof, and ExportProof to return codes.InvalidArgument instead of codes.Unknown for request validation errors. This enables clients to programmatically distinguish bad input from internal errors. Add validation_test.go with tests for invalid input asserts InvalidArgument. Skipped mocking for valid input tests which are already covered by integration tests. Partial fix for Issue #2086 Signed-off-by: kaldun-tech --- docs/release-notes/release-notes-0.8.0.md | 6 + rpcserver/rpcserver.go | 43 ++-- rpcserver/validation_test.go | 251 ++++++++++++++++++++++ 3 files changed, 283 insertions(+), 17 deletions(-) create mode 100644 rpcserver/validation_test.go diff --git a/docs/release-notes/release-notes-0.8.0.md b/docs/release-notes/release-notes-0.8.0.md index d41312a028..d949ff71b7 100644 --- a/docs/release-notes/release-notes-0.8.0.md +++ b/docs/release-notes/release-notes-0.8.0.md @@ -294,6 +294,12 @@ ## RPC Updates +- [PR#2112](https://github.com/lightninglabs/taproot-assets/pull/2112) + `DecodeAddr`, `DecodeProof`, and `ExportProof` now return + `codes.InvalidArgument` for request validation errors instead of + `codes.Unknown`. This enables clients to programmatically distinguish + bad input from internal errors. + - [PR#2005](https://github.com/lightninglabs/taproot-assets/pull/2005) Add a `node_id` field to `QueryAssetRatesRequest` containing the local node's 33-byte compressed public key. This allows the price oracle to diff --git a/rpcserver/rpcserver.go b/rpcserver/rpcserver.go index df88e4bc8c..7a8165bfd4 100644 --- a/rpcserver/rpcserver.go +++ b/rpcserver/rpcserver.go @@ -80,6 +80,8 @@ import ( "golang.org/x/exp/maps" "golang.org/x/time/rate" "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) var ( @@ -2024,12 +2026,14 @@ func (r *RPCServer) DecodeAddr(_ context.Context, req *taprpc.DecodeAddrRequest) (*taprpc.Addr, error) { if len(req.Addr) == 0 { - return nil, fmt.Errorf("must specify an addr") + return nil, status.Error(codes.InvalidArgument, + "must specify an addr") } addr, err := address.DecodeAddress(req.Addr, &r.cfg.ChainParams) if err != nil { - return nil, fmt.Errorf("unable to decode addr: %w", err) + return nil, status.Errorf(codes.InvalidArgument, + "unable to decode addr: %v", err) } rpcAddr, err := marshalAddr(addr, r.cfg.TapAddrBook) @@ -2096,8 +2100,8 @@ func (r *RPCServer) DecodeProof(ctx context.Context, case proof.IsSingleProof(req.RawProof): p, err := proof.Decode(req.RawProof) if err != nil { - return nil, fmt.Errorf("unable to decode proof: %w", - err) + return nil, status.Errorf(codes.InvalidArgument, + "unable to decode proof: %v", err) } rpcProof, err = r.marshalProof( @@ -2112,20 +2116,22 @@ func (r *RPCServer) DecodeProof(ctx context.Context, case proof.IsProofFile(req.RawProof): if err := proof.CheckMaxFileSize(req.RawProof); err != nil { - return nil, fmt.Errorf("invalid proof file: %w", err) + return nil, status.Errorf(codes.InvalidArgument, + "invalid proof file: %v", err) } proofFile, err := proof.DecodeFile(req.RawProof) if err != nil { - return nil, fmt.Errorf("unable to decode proof file: "+ - "%w", err) + return nil, status.Errorf(codes.InvalidArgument, + "unable to decode proof file: %v", err) } latestProofIndex := uint32(proofFile.NumProofs() - 1) if req.ProofAtDepth > latestProofIndex { - return nil, fmt.Errorf("invalid depth %d is greater "+ - "than latest proof index of %d", - req.ProofAtDepth, latestProofIndex) + return nil, status.Errorf(codes.InvalidArgument, + "invalid depth %d is greater than latest "+ + "proof index of %d", req.ProofAtDepth, + latestProofIndex) } // Default to latest proof. @@ -2148,8 +2154,8 @@ func (r *RPCServer) DecodeProof(ctx context.Context, rpcProof.NumberOfProofs = uint32(proofFile.NumProofs()) default: - return nil, fmt.Errorf("invalid raw proof, could not " + - "identify decoding format") + return nil, status.Error(codes.InvalidArgument, + "invalid raw proof, could not identify decoding format") } return &taprpc.DecodeProofResponse{ @@ -2340,16 +2346,19 @@ func (r *RPCServer) ExportProof(ctx context.Context, req *taprpc.ExportProofRequest) (*taprpc.ProofFile, error) { if len(req.ScriptKey) == 0 { - return nil, fmt.Errorf("a valid script key must be specified") + return nil, status.Error(codes.InvalidArgument, + "a valid script key must be specified") } scriptKey, err := rpcutils.ParseUserKey(req.ScriptKey) if err != nil { - return nil, fmt.Errorf("invalid script key: %w", err) + return nil, status.Errorf(codes.InvalidArgument, + "invalid script key: %v", err) } if len(req.AssetId) != 32 { - return nil, fmt.Errorf("asset ID must be 32 bytes") + return nil, status.Error(codes.InvalidArgument, + "asset ID must be 32 bytes") } var ( @@ -2365,8 +2374,8 @@ func (r *RPCServer) ExportProof(ctx context.Context, if req.Outpoint != nil { op, err := rpcutils.UnmarshalOutPoint(req.Outpoint) if err != nil { - return nil, fmt.Errorf("unmarshalling outpoint: %w", - err) + return nil, status.Errorf(codes.InvalidArgument, + "unmarshalling outpoint: %v", err) } outPoint = &op diff --git a/rpcserver/validation_test.go b/rpcserver/validation_test.go new file mode 100644 index 0000000000..d3999677bf --- /dev/null +++ b/rpcserver/validation_test.go @@ -0,0 +1,251 @@ +package rpcserver + +// validation_test.go contains unit tests for RPC input validation. +// These tests verify that invalid input returns codes.InvalidArgument +// (not codes.Unknown). +// +// Happy-path testing (valid input -> successful response) is covered by +// integration tests in itest/ which exercise complete request flows. + +import ( + "context" + "testing" + + "github.com/btcsuite/btcd/btcec/v2" + "github.com/lightninglabs/taproot-assets/address" + "github.com/lightninglabs/taproot-assets/proof" + "github.com/lightninglabs/taproot-assets/tapconfig" + "github.com/lightninglabs/taproot-assets/taprpc" + "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +// assertInvalidArgument checks that the error is a gRPC status error with +// code InvalidArgument and that the message contains the expected substring. +func assertInvalidArgument(t *testing.T, err error, wantMsgContains string) { + t.Helper() + + require.Error(t, err) + + st, ok := status.FromError(err) + require.True(t, ok, "error should be a gRPC status error") + require.Equal(t, codes.InvalidArgument, st.Code(), + "expected InvalidArgument, got %v", st.Code()) + require.Contains(t, st.Message(), wantMsgContains) +} + +// newTestServer creates a minimal RPCServer for validation testing. +func newTestServer() *RPCServer { + return &RPCServer{ + cfg: &tapconfig.Config{ + ChainParams: address.MainNetTap, + }, + } +} + +// TestDecodeAddrValidation tests that DecodeAddr returns InvalidArgument +// for validation errors. +func TestDecodeAddrValidation(t *testing.T) { + t.Parallel() + + server := newTestServer() + + tests := []struct { + name string + req *taprpc.DecodeAddrRequest + wantMsg string + }{ + { + name: "empty address", + req: &taprpc.DecodeAddrRequest{Addr: ""}, + wantMsg: "must specify an addr", + }, + { + name: "invalid address format", + req: &taprpc.DecodeAddrRequest{ + Addr: "not-a-valid-addr", + }, + wantMsg: "unable to decode addr", + }, + { + name: "wrong network prefix", + req: &taprpc.DecodeAddrRequest{ + // Testnet address (taptb1) on mainnet config + // (expects tapbc1). DecodeAddress compares the + // HRP against net.TapHRP and returns + // ErrMismatchedHRP when they don't match. + // See address/address.go ErrMismatchedHRP + Addr: "taptb1qqqszqspqqqqqqqqqqqqqqqqqqqq" + + "qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqpqqq" + + "sqqspqqqqp8hlm7nfnydq5wvs6j5mczq8tf" + + "vemhy7082", + }, + wantMsg: "unable to decode addr", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + _, err := server.DecodeAddr( + context.Background(), tc.req, + ) + assertInvalidArgument(t, err, tc.wantMsg) + }) + } +} + +// TestDecodeProofValidation tests that DecodeProof returns InvalidArgument +// for validation errors. +// +// Note: DecodeProof's validation is inherent to decoding - malformed proofs +// correctly return InvalidArgument. Testing valid input requires a real proof +// fixture; this is covered by integration tests (e.g., itest/proof_test.go). +func TestDecodeProofValidation(t *testing.T) { + t.Parallel() + + server := newTestServer() + + // Create invalid proof bytes that don't match either magic prefix. + invalidMagicBytes := []byte{0x00, 0x00, 0x00, 0x00, 0x01, 0x02, 0x03} + + // Create bytes with single proof magic but invalid content. + invalidSingleProof := append( + proof.PrefixMagicBytes[:], + []byte{0x00, 0x01, 0x02, 0x03}..., + ) + + // Create bytes with file magic but invalid content. + invalidFileProof := append( + proof.FilePrefixMagicBytes[:], + []byte{0x00, 0x01, 0x02, 0x03}..., + ) + + tests := []struct { + name string + req *taprpc.DecodeProofRequest + wantMsg string + }{ + { + // Covers both nil and empty raw proof + name: "empty proof", + req: &taprpc.DecodeProofRequest{RawProof: nil}, + wantMsg: "could not identify decoding format", + }, + { + name: "invalid magic bytes", + req: &taprpc.DecodeProofRequest{ + RawProof: invalidMagicBytes, + }, + wantMsg: "could not identify decoding format", + }, + { + name: "invalid single proof content", + req: &taprpc.DecodeProofRequest{ + RawProof: invalidSingleProof, + }, + wantMsg: "unable to decode proof", + }, + { + name: "invalid file proof content", + req: &taprpc.DecodeProofRequest{ + RawProof: invalidFileProof, + }, + wantMsg: "unable to decode proof file", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + _, err := server.DecodeProof( + context.Background(), tc.req, + ) + assertInvalidArgument(t, err, tc.wantMsg) + }) + } +} + +// TestExportProofValidation tests that ExportProof returns InvalidArgument +// for validation errors. +func TestExportProofValidation(t *testing.T) { + t.Parallel() + + server := newTestServer() + + // Generate a valid compressed public key for testing. + privKey, err := btcec.NewPrivateKey() + require.NoError(t, err) + validScriptKey := privKey.PubKey().SerializeCompressed() + + tests := []struct { + name string + req *taprpc.ExportProofRequest + wantMsg string + }{ + { + name: "empty script key", + req: &taprpc.ExportProofRequest{ScriptKey: nil}, + wantMsg: "a valid script key must be specified", + }, + { + name: "invalid script key - wrong length", + req: &taprpc.ExportProofRequest{ + ScriptKey: []byte{0x01, 0x02, 0x03}, + }, + wantMsg: "invalid script key", + }, + { + name: "invalid script key - bad prefix", + req: &taprpc.ExportProofRequest{ + ScriptKey: make([]byte, 33), // all zeros + }, + wantMsg: "invalid script key", + }, + { + name: "asset ID wrong length - empty", + req: &taprpc.ExportProofRequest{ + ScriptKey: validScriptKey, + AssetId: nil, + }, + wantMsg: "asset ID must be 32 bytes", + }, + { + name: "asset ID wrong length - too short", + req: &taprpc.ExportProofRequest{ + ScriptKey: validScriptKey, + AssetId: make([]byte, 31), + }, + wantMsg: "asset ID must be 32 bytes", + }, + { + name: "asset ID wrong length - too long", + req: &taprpc.ExportProofRequest{ + ScriptKey: validScriptKey, + AssetId: make([]byte, 33), + }, + wantMsg: "asset ID must be 32 bytes", + }, + { + name: "invalid outpoint - bad txid", + req: &taprpc.ExportProofRequest{ + ScriptKey: validScriptKey, + AssetId: make([]byte, 32), + Outpoint: &taprpc.OutPoint{ + // Wrong length for txid. + Txid: []byte{0x01, 0x02}, + OutputIndex: 0, + }, + }, + wantMsg: "unmarshalling outpoint", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + _, err := server.ExportProof( + context.Background(), tc.req, + ) + assertInvalidArgument(t, err, tc.wantMsg) + }) + } +} From ae266640004f0008c2d597490dd39d7f409568ec Mon Sep 17 00:00:00 2001 From: kaldun-tech Date: Fri, 8 May 2026 12:14:32 -0600 Subject: [PATCH 2/2] rpcserver: simplify validation tests per review Replace wantMsg with wantCode and use assertCode helper to check only the gRPC status code, not the error message. This makes tests less brittle and more focused on the programmatic error code. Signed-off-by: kaldun-tech --- rpcserver/validation_test.go | 134 +++++++++++++++-------------------- 1 file changed, 59 insertions(+), 75 deletions(-) diff --git a/rpcserver/validation_test.go b/rpcserver/validation_test.go index d3999677bf..b65ed75580 100644 --- a/rpcserver/validation_test.go +++ b/rpcserver/validation_test.go @@ -21,18 +21,16 @@ import ( "google.golang.org/grpc/status" ) -// assertInvalidArgument checks that the error is a gRPC status error with -// code InvalidArgument and that the message contains the expected substring. -func assertInvalidArgument(t *testing.T, err error, wantMsgContains string) { +// assertCode checks that the error is a gRPC status error with the expected +// status code. +func assertCode(t *testing.T, err error, wantCode codes.Code) { t.Helper() require.Error(t, err) st, ok := status.FromError(err) require.True(t, ok, "error should be a gRPC status error") - require.Equal(t, codes.InvalidArgument, st.Code(), - "expected InvalidArgument, got %v", st.Code()) - require.Contains(t, st.Message(), wantMsgContains) + require.Equal(t, wantCode, st.Code()) } // newTestServer creates a minimal RPCServer for validation testing. @@ -52,115 +50,101 @@ func TestDecodeAddrValidation(t *testing.T) { server := newTestServer() tests := []struct { - name string - req *taprpc.DecodeAddrRequest - wantMsg string + name string + req *taprpc.DecodeAddrRequest + wantCode codes.Code }{ { - name: "empty address", - req: &taprpc.DecodeAddrRequest{Addr: ""}, - wantMsg: "must specify an addr", + name: "empty address", + req: &taprpc.DecodeAddrRequest{Addr: ""}, + wantCode: codes.InvalidArgument, }, { - name: "invalid address format", - req: &taprpc.DecodeAddrRequest{ - Addr: "not-a-valid-addr", - }, - wantMsg: "unable to decode addr", + name: "invalid address format", + req: &taprpc.DecodeAddrRequest{Addr: "not-valid"}, + wantCode: codes.InvalidArgument, }, { name: "wrong network prefix", req: &taprpc.DecodeAddrRequest{ - // Testnet address (taptb1) on mainnet config - // (expects tapbc1). DecodeAddress compares the - // HRP against net.TapHRP and returns - // ErrMismatchedHRP when they don't match. - // See address/address.go ErrMismatchedHRP - Addr: "taptb1qqqszqspqqqqqqqqqqqqqqqqqqqq" + - "qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqpqqq" + + // Testnet address on mainnet config. + Addr: "taptb1qqqszqspqqqqqqqqqqqqqqqqqqq" + + "qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqpqqq" + "sqqspqqqqp8hlm7nfnydq5wvs6j5mczq8tf" + "vemhy7082", }, - wantMsg: "unable to decode addr", + wantCode: codes.InvalidArgument, }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { _, err := server.DecodeAddr( - context.Background(), tc.req, - ) - assertInvalidArgument(t, err, tc.wantMsg) + context.Background(), tc.req) + assertCode(t, err, tc.wantCode) }) } } // TestDecodeProofValidation tests that DecodeProof returns InvalidArgument // for validation errors. -// -// Note: DecodeProof's validation is inherent to decoding - malformed proofs -// correctly return InvalidArgument. Testing valid input requires a real proof -// fixture; this is covered by integration tests (e.g., itest/proof_test.go). func TestDecodeProofValidation(t *testing.T) { t.Parallel() server := newTestServer() - // Create invalid proof bytes that don't match either magic prefix. + // Invalid proof bytes that don't match either magic prefix. invalidMagicBytes := []byte{0x00, 0x00, 0x00, 0x00, 0x01, 0x02, 0x03} - // Create bytes with single proof magic but invalid content. + // Bytes with single proof magic but invalid content. invalidSingleProof := append( - proof.PrefixMagicBytes[:], - []byte{0x00, 0x01, 0x02, 0x03}..., + proof.PrefixMagicBytes[:], []byte{0x00, 0x01, 0x02, 0x03}..., ) - // Create bytes with file magic but invalid content. + // Bytes with file magic but invalid content. invalidFileProof := append( proof.FilePrefixMagicBytes[:], []byte{0x00, 0x01, 0x02, 0x03}..., ) tests := []struct { - name string - req *taprpc.DecodeProofRequest - wantMsg string + name string + req *taprpc.DecodeProofRequest + wantCode codes.Code }{ { - // Covers both nil and empty raw proof - name: "empty proof", - req: &taprpc.DecodeProofRequest{RawProof: nil}, - wantMsg: "could not identify decoding format", + name: "empty proof", + req: &taprpc.DecodeProofRequest{RawProof: nil}, + wantCode: codes.InvalidArgument, }, { name: "invalid magic bytes", req: &taprpc.DecodeProofRequest{ RawProof: invalidMagicBytes, }, - wantMsg: "could not identify decoding format", + wantCode: codes.InvalidArgument, }, { - name: "invalid single proof content", + name: "invalid single proof", req: &taprpc.DecodeProofRequest{ RawProof: invalidSingleProof, }, - wantMsg: "unable to decode proof", + wantCode: codes.InvalidArgument, }, { - name: "invalid file proof content", + name: "invalid file proof", req: &taprpc.DecodeProofRequest{ RawProof: invalidFileProof, }, - wantMsg: "unable to decode proof file", + wantCode: codes.InvalidArgument, }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { _, err := server.DecodeProof( - context.Background(), tc.req, - ) - assertInvalidArgument(t, err, tc.wantMsg) + context.Background(), tc.req) + assertCode(t, err, tc.wantCode) }) } } @@ -178,74 +162,74 @@ func TestExportProofValidation(t *testing.T) { validScriptKey := privKey.PubKey().SerializeCompressed() tests := []struct { - name string - req *taprpc.ExportProofRequest - wantMsg string + name string + req *taprpc.ExportProofRequest + wantCode codes.Code }{ { - name: "empty script key", - req: &taprpc.ExportProofRequest{ScriptKey: nil}, - wantMsg: "a valid script key must be specified", + name: "empty script key", + req: &taprpc.ExportProofRequest{ScriptKey: nil}, + wantCode: codes.InvalidArgument, }, { - name: "invalid script key - wrong length", + name: "invalid script key length", req: &taprpc.ExportProofRequest{ ScriptKey: []byte{0x01, 0x02, 0x03}, }, - wantMsg: "invalid script key", + wantCode: codes.InvalidArgument, }, { - name: "invalid script key - bad prefix", + name: "invalid script key prefix", req: &taprpc.ExportProofRequest{ - ScriptKey: make([]byte, 33), // all zeros + // 33 bytes is correct length for compressed + // pubkey, but 0x00 prefix is invalid. + ScriptKey: make([]byte, 33), }, - wantMsg: "invalid script key", + wantCode: codes.InvalidArgument, }, { - name: "asset ID wrong length - empty", + name: "empty asset ID", req: &taprpc.ExportProofRequest{ ScriptKey: validScriptKey, AssetId: nil, }, - wantMsg: "asset ID must be 32 bytes", + wantCode: codes.InvalidArgument, }, { - name: "asset ID wrong length - too short", + name: "asset ID too short", req: &taprpc.ExportProofRequest{ ScriptKey: validScriptKey, AssetId: make([]byte, 31), }, - wantMsg: "asset ID must be 32 bytes", + wantCode: codes.InvalidArgument, }, { - name: "asset ID wrong length - too long", + name: "asset ID too long", req: &taprpc.ExportProofRequest{ ScriptKey: validScriptKey, AssetId: make([]byte, 33), }, - wantMsg: "asset ID must be 32 bytes", + wantCode: codes.InvalidArgument, }, { - name: "invalid outpoint - bad txid", + name: "invalid outpoint", req: &taprpc.ExportProofRequest{ ScriptKey: validScriptKey, AssetId: make([]byte, 32), Outpoint: &taprpc.OutPoint{ - // Wrong length for txid. Txid: []byte{0x01, 0x02}, OutputIndex: 0, }, }, - wantMsg: "unmarshalling outpoint", + wantCode: codes.InvalidArgument, }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { _, err := server.ExportProof( - context.Background(), tc.req, - ) - assertInvalidArgument(t, err, tc.wantMsg) + context.Background(), tc.req) + assertCode(t, err, tc.wantCode) }) } }