From 6ef7e752515f26e5d90b7865a8622605a448c00b Mon Sep 17 00:00:00 2001 From: George Date: Tue, 20 Aug 2024 10:50:48 -0700 Subject: [PATCH 1/6] Add more details to panics --- cmd/soroban-rpc/lib/xdr2json/src/lib.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/cmd/soroban-rpc/lib/xdr2json/src/lib.rs b/cmd/soroban-rpc/lib/xdr2json/src/lib.rs index c4df4c62..e1f524be 100644 --- a/cmd/soroban-rpc/lib/xdr2json/src/lib.rs +++ b/cmd/soroban-rpc/lib/xdr2json/src/lib.rs @@ -50,12 +50,18 @@ pub unsafe extern "C" fn xdr_to_json( ) -> *mut ConversionResult { let result = catch_json_to_xdr_panic(Box::new(move || { let type_str = unsafe { from_c_string(typename) }; - let the_type = xdr::TypeVariant::from_str(&type_str).unwrap(); + let the_type = match xdr::TypeVariant::from_str(&type_str) { + Ok(t) => t, + Err(e) => panic!("couldn't match type {type_str}: {}", e), + }; let xdr_bytearray = unsafe { from_c_xdr(xdr) }; let mut buffer = xdr::Limited::new(xdr_bytearray.as_slice(), DEFAULT_XDR_RW_LIMITS.clone()); - let t = xdr::Type::read_xdr_to_end(the_type, &mut buffer).unwrap(); + let t = match xdr::Type::read_xdr_to_end(the_type, &mut buffer) { + Ok(t) => t, + Err(e) => panic!("couldn't read {type_str}: {}", e), + }; Ok(RustConversionResult { json: serde_json::to_string(&t).unwrap(), From 323c21f9a8cd69ba241c0a85f8ff15d925fd6f66 Mon Sep 17 00:00:00 2001 From: George Date: Tue, 20 Aug 2024 10:51:03 -0700 Subject: [PATCH 2/6] Omit encoding txdata on errors --- .../internal/methods/simulate_transaction.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/cmd/soroban-rpc/internal/methods/simulate_transaction.go b/cmd/soroban-rpc/internal/methods/simulate_transaction.go index 2a26b08a..0fc318c2 100644 --- a/cmd/soroban-rpc/internal/methods/simulate_transaction.go +++ b/cmd/soroban-rpc/internal/methods/simulate_transaction.go @@ -398,13 +398,15 @@ func NewSimulateTransactionHandler(logger *log.Entry, ledgerEntryReader db.Ledge switch request.Format { case FormatJSON: - simResp.TransactionDataJSON, err = xdr2json.ConvertBytes( - xdr.SorobanTransactionData{}, - result.TransactionData) - if err != nil { - return SimulateTransactionResponse{ - Error: err.Error(), - LatestLedger: latestLedger, + if len(result.TransactionData) > 0 { + simResp.TransactionDataJSON, err = xdr2json.ConvertBytes( + xdr.SorobanTransactionData{}, + result.TransactionData) + if err != nil { + return SimulateTransactionResponse{ + Error: err.Error(), + LatestLedger: latestLedger, + } } } From c74de5d5536d8b20162daf23ddeaf89c0c2b46bc Mon Sep 17 00:00:00 2001 From: George Date: Tue, 20 Aug 2024 10:53:26 -0700 Subject: [PATCH 3/6] Generalize errors: string,omitempty always kicks in --- .../internal/methods/simulate_transaction.go | 16 +++++++--------- cmd/soroban-rpc/internal/xdr2json/conversion.go | 12 ++++++++---- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/cmd/soroban-rpc/internal/methods/simulate_transaction.go b/cmd/soroban-rpc/internal/methods/simulate_transaction.go index 0fc318c2..2a26b08a 100644 --- a/cmd/soroban-rpc/internal/methods/simulate_transaction.go +++ b/cmd/soroban-rpc/internal/methods/simulate_transaction.go @@ -398,15 +398,13 @@ func NewSimulateTransactionHandler(logger *log.Entry, ledgerEntryReader db.Ledge switch request.Format { case FormatJSON: - if len(result.TransactionData) > 0 { - simResp.TransactionDataJSON, err = xdr2json.ConvertBytes( - xdr.SorobanTransactionData{}, - result.TransactionData) - if err != nil { - return SimulateTransactionResponse{ - Error: err.Error(), - LatestLedger: latestLedger, - } + simResp.TransactionDataJSON, err = xdr2json.ConvertBytes( + xdr.SorobanTransactionData{}, + result.TransactionData) + if err != nil { + return SimulateTransactionResponse{ + Error: err.Error(), + LatestLedger: latestLedger, } } diff --git a/cmd/soroban-rpc/internal/xdr2json/conversion.go b/cmd/soroban-rpc/internal/xdr2json/conversion.go index 828b3145..7992ab9b 100644 --- a/cmd/soroban-rpc/internal/xdr2json/conversion.go +++ b/cmd/soroban-rpc/internal/xdr2json/conversion.go @@ -27,20 +27,24 @@ import ( // and returns the raw JSON-formatted serialization of that object. // It can be unmarshalled to a proper JSON structure, but the raw bytes are // returned to avoid unnecessary round-trips. If there is an -// error, it returns an empty JSON object. +// error, it returns an empty string. // // The `xdr` object does not need to actually be initialized/valid: // we only use it to determine the name of the structure. We could just // accept a string, but that would make mistakes likelier than passing the // structure itself (by reference). -func ConvertBytes(xdr interface{}, field []byte) ([]byte, error) { +func ConvertBytes(xdr interface{}, field []byte) (json.RawMessage, error) { + if len(field) == 0 { + return []byte(""), nil + } + xdrTypeName := reflect.TypeOf(xdr).Name() return convertAnyBytes(xdrTypeName, field) } // ConvertInterface takes a valid XDR object (`xdr`) and returns // the raw JSON-formatted serialization of that object. If there is an -// error, it returns an empty JSON object. +// error, it returns an empty string. // // Unlike `ConvertBytes`, the value here needs to be valid and // serializable. @@ -48,7 +52,7 @@ func ConvertInterface(xdr encoding.BinaryMarshaler) (json.RawMessage, error) { xdrTypeName := reflect.TypeOf(xdr).Name() data, err := xdr.MarshalBinary() if err != nil { - return []byte("{}"), errors.Wrapf(err, "failed to serialize XDR type '%s'", xdrTypeName) + return []byte(""), errors.Wrapf(err, "failed to serialize XDR type '%s'", xdrTypeName) } return convertAnyBytes(xdrTypeName, data) From 86f58b2a8e0e6891548727692df86e2d10125c93 Mon Sep 17 00:00:00 2001 From: George Date: Fri, 23 Aug 2024 11:57:38 -0700 Subject: [PATCH 4/6] Add test cases for conversion library --- .../internal/xdr2json/conversion_test.go | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 cmd/soroban-rpc/internal/xdr2json/conversion_test.go diff --git a/cmd/soroban-rpc/internal/xdr2json/conversion_test.go b/cmd/soroban-rpc/internal/xdr2json/conversion_test.go new file mode 100644 index 00000000..0c0da02d --- /dev/null +++ b/cmd/soroban-rpc/internal/xdr2json/conversion_test.go @@ -0,0 +1,45 @@ +package xdr2json + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/stellar/go/keypair" + "github.com/stellar/go/xdr" +) + +func TestConversion(t *testing.T) { + // Make a structure to encode + pubkey := keypair.MustRandom() + asset := xdr.MustNewCreditAsset("ABCD", pubkey.Address()) + + // Try the all-inclusive version + jsi, err := ConvertInterface(asset) + require.NoError(t, err) + + // Try the byte-and-interface version + rawBytes, err := asset.MarshalBinary() + require.NoError(t, err) + jsb, err := ConvertBytes(xdr.Asset{}, rawBytes) + require.NoError(t, err) + + for _, rawJs := range []json.RawMessage{jsi, jsb} { + var dest map[string]interface{} + require.NoError(t, json.Unmarshal(rawJs, &dest)) + + require.Contains(t, dest, "credit_alphanum4") + require.Contains(t, dest["credit_alphanum4"], "asset_code") + require.Contains(t, dest["credit_alphanum4"], "issuer") + require.IsType(t, dest["credit_alphanum4"], map[string]interface{}{}) + require.Equal(t, pubkey.Address(), + dest["credit_alphanum4"].(map[string]interface{})["issuer"]) + } +} + +func TestEmptyConversion(t *testing.T) { + js, err := ConvertBytes(xdr.SorobanTransactionData{}, []byte{}) + require.NoError(t, err) + require.Equal(t, "", string(js)) +} From b283a60f840e5b99405ae3d357f31a2ab9f993a6 Mon Sep 17 00:00:00 2001 From: George Date: Fri, 23 Aug 2024 12:04:31 -0700 Subject: [PATCH 5/6] Omg linter fuck off --- cmd/soroban-rpc/internal/xdr2json/conversion_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/soroban-rpc/internal/xdr2json/conversion_test.go b/cmd/soroban-rpc/internal/xdr2json/conversion_test.go index 0c0da02d..cf014439 100644 --- a/cmd/soroban-rpc/internal/xdr2json/conversion_test.go +++ b/cmd/soroban-rpc/internal/xdr2json/conversion_test.go @@ -32,7 +32,7 @@ func TestConversion(t *testing.T) { require.Contains(t, dest, "credit_alphanum4") require.Contains(t, dest["credit_alphanum4"], "asset_code") require.Contains(t, dest["credit_alphanum4"], "issuer") - require.IsType(t, dest["credit_alphanum4"], map[string]interface{}{}) + require.IsType(t, map[string]interface{}{}, dest["credit_alphanum4"]) require.Equal(t, pubkey.Address(), dest["credit_alphanum4"].(map[string]interface{})["issuer"]) } From 1e583e3a40b0756fd03088d7eaf793bd1d9f3900 Mon Sep 17 00:00:00 2001 From: George Date: Tue, 27 Aug 2024 15:52:07 -0700 Subject: [PATCH 6/6] Linter too stupid to realize I've checked the type already --- cmd/soroban-rpc/internal/xdr2json/conversion_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cmd/soroban-rpc/internal/xdr2json/conversion_test.go b/cmd/soroban-rpc/internal/xdr2json/conversion_test.go index cf014439..933eec40 100644 --- a/cmd/soroban-rpc/internal/xdr2json/conversion_test.go +++ b/cmd/soroban-rpc/internal/xdr2json/conversion_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/stellar/go/keypair" @@ -33,8 +34,9 @@ func TestConversion(t *testing.T) { require.Contains(t, dest["credit_alphanum4"], "asset_code") require.Contains(t, dest["credit_alphanum4"], "issuer") require.IsType(t, map[string]interface{}{}, dest["credit_alphanum4"]) - require.Equal(t, pubkey.Address(), - dest["credit_alphanum4"].(map[string]interface{})["issuer"]) + if converted, ok := dest["credit_alphanum4"].(map[string]interface{}); assert.True(t, ok) { + require.Equal(t, pubkey.Address(), converted["issuer"]) + } } }