Go Request Builder Library #265
Conversation
WalkthroughThe recent updates introduce several new adaptor call builders and accompanying test functions, focusing on various DeFi protocols like Aave, Balancer, and Uniswap. New functionalities for handling assets and contracts within these protocols have been added, including operations like depositing, withdrawing, and managing debt tokens. Additionally, example files have been provided to demonstrate the usage of these new builders and methods in real-world scenarios. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ExampleBroadcastRequests
participant SommelierChain
participant StewardInstance
User->>ExampleBroadcastRequests: Call Function
ExampleBroadcastRequests->>SommelierChain: Query for Steward Instances
SommelierChain-->>ExampleBroadcastRequests: Return List of Instances
loop For each StewardInstance
ExampleBroadcastRequests->>StewardInstance: Send Request
StewardInstance-->>ExampleBroadcastRequests: Acknowledge Request
end
ExampleBroadcastRequests-->>User: Complete
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Outside diff range, codebase verification and nitpick comments (9)
go/builder/examples/create_tls_client.go (1)
13-31: Consider adding logging for better debugging.Adding logging statements can help in debugging issues related to loading certificates and building credentials.
import ( "log" ) // Add logging statements log.Printf("Loading client certificate from %s and %s", clientCertPath, clientKeyPath) clientCert, err := tls.LoadX509KeyPair(clientCertPath, clientKeyPath) if err != nil { log.Printf("Error loading client certificate: %v", err) return nil, err } log.Printf("Reading server CA from %s", serverCAPath) serverCA, err := os.ReadFile(serverCAPath) if err != nil { log.Printf("Error reading server CA: %v", err) return nil, err } rootPool := x509.NewCertPool() rootPool.AppendCertsFromPEM(serverCA) tlsConfig := &tls.Config{ Certificates: []tls.Certificate{clientCert}, RootCAs: rootPool, } log.Println("TLS credentials successfully built") return credentials.NewTLS(tlsConfig), nilgo/builder/examples/check_steward_status.go (1)
35-51: Consider adding logging for better debugging.Adding logging statements can help in debugging issues related to checking the server status.
import ( "log" ) // Add logging statements log.Println("Creating TLS status client") client, err := CreateTlsStatusClient() if err != nil { log.Fatalf("Error creating TLS status client: %v", err) } log.Println("Creating context with timeout") ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() log.Println("Sending version request") response, err := client.Version(ctx, &steward_proto.VersionRequest{}) if err != nil { log.Fatalf("Error sending version request: %v", err) } log.Println("Received response") fmt.Print(response)go/builder/examples/adaptor_call.go (1)
14-58: Consider adding logging for better debugging.Adding logging statements can help in debugging issues related to making the adaptor call.
import ( "log" ) // Add logging statements log.Println("Creating TLS client") client, err := CreateTlsClient() if err != nil { log.Fatalf("Error creating TLS client: %v", err) } log.Println("Creating context with timeout") ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() log.Println("Building adaptor call") adaptor := common.HexToAddress("0x1234567890000000000000000000000000000000") token := common.HexToAddress("0x7Fc66500c84A76Ad7e9c93437bFc5Ac33E2DDaE9") amount := big.NewInt(100000) adaptorCall := adaptors.NewAaveV2ATokenAdaptorV2CallBuilder(adaptor). DepositToAave(token, amount). Build() log.Println("Building call data") callData, err := builder.NewCellarCallDataBuilder(). CallOnAdaptor(adaptorCall). Build() if err != nil { log.Fatalf("Error building call data: %v", err) } log.Println("Building request") cellarId := common.HexToAddress("0x0") request, err := builder.NewScheduleRequestBuilder(). WithCellarID(cellarId). WithChainID(1). WithCallData(callData). WithBlockHeight(100). Build() if err != nil { log.Fatalf("Error building request: %v", err) } log.Println("Sending request") response, err := client.Schedule(ctx, request) if err != nil { log.Fatalf("Error sending request: %v", err) } log.Println("Received response") fmt.Print(response)go/builder/examples/multicall.go (2)
13-35: Ensure proper error handling and logging.The function builds a multicall request and handles errors by returning them. Consider adding logging for better traceability and debugging.
if err != nil { log.Printf("Error building call data: %v", err) return nil, err }
38-61: Ensure proper error handling and logging.The function demonstrates sending a multicall request. Consider adding logging for better traceability and debugging.
if err != nil { log.Printf("Error creating TLS client: %v", err) panic(err) }go/builder/adaptors/aave_v3_a_token_test.go (3)
12-20: Ensure comprehensive test coverage.The test checks the constructor for
AaveV3ATokenAdaptorV1CallBuilder. Consider adding more assertions to verify the internal state of the builder.assert.NotNil(t, builder) assert.Equal(t, adaptor, builder.adaptor)
22-32: Ensure comprehensive test coverage.The test checks the
DepositToAavefunction. Consider adding assertions to verify the parameters of the function call.call := builder.calls[0].Function.(*steward_proto.AaveV3ATokenAdaptorV1_DepositToAave_) assert.Equal(t, asset.Hex(), call.DepositToAave.Asset) assert.Equal(t, "100", call.DepositToAave.Amount.String())
34-44: Ensure comprehensive test coverage.The test checks the
WithdrawFromAavefunction. Consider adding assertions to verify the parameters of the function call.call := builder.calls[0].Function.(*steward_proto.AaveV3ATokenAdaptorV1_WithdrawFromAave_) assert.Equal(t, asset.Hex(), call.WithdrawFromAave.Asset) assert.Equal(t, "100", call.WithdrawFromAave.Amount.String())go/builder/examples/broadcast_requests.go (1)
11-50: Ensure proper error handling and logging.The function demonstrates broadcasting requests. Consider adding logging for better traceability and debugging.
if err != nil { log.Printf("Error querying subscribers: %v", err) panic(err) }
| assert.Error(t, error) | ||
| } | ||
|
|
||
| // Test the CallDataBuilder |
There was a problem hiding this comment.
Implement the missing test function or remove the placeholder.
The placeholder for the TestCallDataBuilder function is present but not implemented.
Do you want me to implement the missing test function or remove the placeholder?
go/builder/cellar_call_test.go
Outdated
| // Test the CallDataBuilder constructor | ||
| func TestNewCallData(t *testing.T) { | ||
| // Create a new CellarCallBuilder | ||
| builder := NewCellarCallDataBuilder() | ||
|
|
||
| // Check the builder | ||
| assert.Equal(t, 0, len(builder.functionCalls)) | ||
|
|
||
| // Can't build an empty builder | ||
| result, error := builder.Build() | ||
| assert.Nil(t, result) | ||
| assert.Error(t, error) | ||
| } |
There was a problem hiding this comment.
Update function name and comments to reflect new builder name.
The function name and comments should be updated to reflect the new CellarCallDataBuilder name.
- // Test the CallDataBuilder constructor
+ // Test the CellarCallDataBuilder constructor
func TestNewCallData(t *testing.T) {
// Create a new CellarCallBuilder
- builder := NewCellarCallDataBuilder()
+ builder := NewCellarCallDataBuilder()
// Check the builder
assert.Equal(t, 0, len(builder.functionCalls))
// Can't build an empty builder
result, error := builder.Build()
assert.Nil(t, result)
assert.Error(t, error)
}Committable suggestion was skipped due to low confidence.
| func CreateInsecureSimulateClient() { | ||
| addr := "localhost:5734" | ||
| conn, err := grpc.NewClient(addr, grpc.WithTransportCredentials(insecure.NewCredentials())) | ||
| if err != nil { | ||
| panic(err) | ||
| } | ||
|
|
||
| defer conn.Close() | ||
|
|
||
| client := steward_proto.NewSimulateContractCallServiceClient(conn) | ||
| ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) | ||
| defer cancel() | ||
|
|
||
| // Use the client | ||
| client.Simulate(ctx, nil) |
There was a problem hiding this comment.
Add error handling and parameterize the address.
The function lacks error handling for the simulation call and the address should be parameterized for better flexibility.
func CreateInsecureSimulateClient(addr string) {
- addr := "localhost:5734"
conn, err := grpc.NewClient(addr, grpc.WithTransportCredentials(insecure.NewCredentials()))
if err != nil {
panic(err)
}
defer conn.Close()
client := steward_proto.NewSimulateContractCallServiceClient(conn)
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
defer cancel()
// Use the client
- client.Simulate(ctx, nil)
+ resp, err := client.Simulate(ctx, nil)
+ if err != nil {
+ panic(err)
+ }
+ fmt.Println(resp)
}Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func CreateInsecureSimulateClient() { | |
| addr := "localhost:5734" | |
| conn, err := grpc.NewClient(addr, grpc.WithTransportCredentials(insecure.NewCredentials())) | |
| if err != nil { | |
| panic(err) | |
| } | |
| defer conn.Close() | |
| client := steward_proto.NewSimulateContractCallServiceClient(conn) | |
| ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) | |
| defer cancel() | |
| // Use the client | |
| client.Simulate(ctx, nil) | |
| func CreateInsecureSimulateClient(addr string) { | |
| conn, err := grpc.NewClient(addr, grpc.WithTransportCredentials(insecure.NewCredentials())) | |
| if err != nil { | |
| panic(err) | |
| } | |
| defer conn.Close() | |
| client := steward_proto.NewSimulateContractCallServiceClient(conn) | |
| ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) | |
| defer cancel() | |
| // Use the client | |
| resp, err := client.Simulate(ctx, nil) | |
| if err != nil { | |
| panic(err) | |
| } | |
| fmt.Println(resp) | |
| } |
| // Test enabling an asset as collateral | ||
| func TestEnableAssetAsCollateral(t *testing.T) { | ||
| adaptor := common.HexToAddress("0x1234567890123456789012345678901234567890") | ||
| builder := NewAaveV2EnableAssetAsCollateralAdaptorV1CallBuilder(adaptor) | ||
|
|
||
| // Test enabling | ||
| asset := common.HexToAddress("0x00000000000000000000000000000000000000000") | ||
| builder.SetUserUseReserveAsCollateral(asset, true) | ||
|
|
||
| // Check the builder | ||
| assert.Equal(t, 1, len(builder.calls)) | ||
| assert.IsType(t, &steward_proto.AaveV2EnableAssetAsCollateralAdaptorV1_SetUserUseReserveAsCollateral_{}, builder.calls[0].Function) | ||
| } |
There was a problem hiding this comment.
Update the asset address to a valid one.
The asset address used in the test is invalid.
asset := common.HexToAddress("0x00000000000000000000000000000000000000000")should be updated to a valid address.
asset := common.HexToAddress("0x0000000000000000000000000000000000000000")| // Test RepayDebt function | ||
| func TestRepayDebt(t *testing.T) { | ||
| adaptor := common.HexToAddress("0x1234567890123456789012345678901234567890") | ||
| builder := NewAaveV3DebtTokenAdaptorV1CallBuilder(adaptor) | ||
|
|
||
| asset := common.HexToAddress("0x00000000000000000000000000000000000000000") | ||
| builder.RepayAaveDebt(asset, big.NewInt(100)) | ||
|
|
||
| assert.Equal(t, 1, len(builder.calls)) | ||
| assert.IsType(t, &steward_proto.AaveV3DebtTokenAdaptorV1_RepayAaveDebt_{}, builder.calls[0].Function) | ||
| } |
There was a problem hiding this comment.
Update the asset address to a valid one.
The asset address used in the test is invalid.
asset := common.HexToAddress("0x00000000000000000000000000000000000000000")should be updated to a valid address.
asset := common.HexToAddress("0x0000000000000000000000000000000000000000")| func CreateTlsClient() (steward_proto.ContractCallServiceClient, error) { | ||
| // This example uses fake file paths for the auth materials | ||
| creds, err := buildCredentials("client.crt", "client.key", "server_ca.crt") | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| addr := "localhost:5734" | ||
| conn, err := grpc.NewClient(addr, grpc.WithTransportCredentials(creds)) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| defer conn.Close() | ||
|
|
||
| client := steward_proto.NewContractCallServiceClient(conn) | ||
|
|
||
| return client, nil |
There was a problem hiding this comment.
Improve error handling by providing more context.
Providing more context in error messages can help in identifying the root cause of the issue.
creds, err := buildCredentials("client.crt", "client.key", "server_ca.crt")
if err != nil {
return nil, fmt.Errorf("failed to build credentials: %w", err)
}
conn, err := grpc.NewClient(addr, grpc.WithTransportCredentials(creds))
if err != nil {
return nil, fmt.Errorf("failed to create gRPC client: %w", err)
}Consider moving the connection close logic to the caller.
Closing the connection immediately after creating the client might lead to unexpected behavior. Consider moving the connection close logic to the caller.
func CreateTlsClient() (steward_proto.ContractCallServiceClient, *grpc.ClientConn, error) {
creds, err := buildCredentials("client.crt", "client.key", "server_ca.crt")
if err != nil {
return nil, nil, fmt.Errorf("failed to build credentials: %w", err)
}
addr := "localhost:5734"
conn, err := grpc.NewClient(addr, grpc.WithTransportCredentials(creds))
if err != nil {
return nil, nil, fmt.Errorf("failed to create gRPC client: %w", err)
}
client := steward_proto.NewContractCallServiceClient(conn)
return client, conn, nil
}
// Caller should close the connection
client, conn, err := CreateTlsClient()
if err != nil {
log.Fatalf("Error creating TLS client: %v", err)
}
defer conn.Close()| func CreateTlsStatusClient() (steward_proto.StatusServiceClient, error) { | ||
| // This example uses fake file paths for the auth materials | ||
| creds, err := buildCredentials("client.crt", "client.key", "server_ca.crt") | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| addr := "localhost:5734" | ||
| conn, err := grpc.NewClient(addr, grpc.WithTransportCredentials(creds)) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| defer conn.Close() | ||
|
|
||
| client := steward_proto.NewStatusServiceClient(conn) | ||
|
|
||
| return client, nil |
There was a problem hiding this comment.
Improve error handling by providing more context.
Providing more context in error messages can help in identifying the root cause of the issue.
creds, err := buildCredentials("client.crt", "client.key", "server_ca.crt")
if err != nil {
return nil, fmt.Errorf("failed to build credentials: %w", err)
}
conn, err := grpc.NewClient(addr, grpc.WithTransportCredentials(creds))
if err != nil {
return nil, fmt.Errorf("failed to create gRPC client: %w", err)
}Consider moving the connection close logic to the caller.
Closing the connection immediately after creating the client might lead to unexpected behavior. Consider moving the connection close logic to the caller.
func CreateTlsStatusClient() (steward_proto.StatusServiceClient, *grpc.ClientConn, error) {
creds, err := buildCredentials("client.crt", "client.key", "server_ca.crt")
if err != nil {
return nil, nil, fmt.Errorf("failed to build credentials: %w", err)
}
addr := "localhost:5734"
conn, err := grpc.NewClient(addr, grpc.WithTransportCredentials(creds))
if err != nil {
return nil, nil, fmt.Errorf("failed to create gRPC client: %w", err)
}
client := steward_proto.NewStatusServiceClient(conn)
return client, conn, nil
}
// Caller should close the connection
client, conn, err := CreateTlsStatusClient()
if err != nil {
log.Fatalf("Error creating TLS status client: %v", err)
}
defer conn.Close()| func ExampleCheckStewardStatus() { | ||
| // Get client and context | ||
| client, err := CreateTlsStatusClient() | ||
| if err != nil { | ||
| panic(err) | ||
| } | ||
|
|
||
| ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) | ||
| defer cancel() | ||
|
|
||
| // Send request | ||
| response, err := client.Version(ctx, &steward_proto.VersionRequest{}) | ||
| if err != nil { | ||
| panic(err) | ||
| } | ||
|
|
||
| fmt.Print(response) |
There was a problem hiding this comment.
Improve error handling by providing more context.
Providing more context in error messages can help in identifying the root cause of the issue.
client, err := CreateTlsStatusClient()
if err != nil {
log.Fatalf("Error creating TLS status client: %v", err)
}
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()
response, err := client.Version(ctx, &steward_proto.VersionRequest{})
if err != nil {
log.Fatalf("Error sending version request: %v", err)
}| // Test RequestFlashLoan function | ||
| func TestRequestFlashLoan(t *testing.T) { | ||
| adaptor := common.HexToAddress("0x1234567890123456789012345678901234567890") | ||
| builder := NewAaveV3DebtTokenFlashLoanAdaptorV1CallBuilder(adaptor) | ||
|
|
||
| asset := common.HexToAddress("0x00000000000000000000000000000000000000000") | ||
|
|
||
| // AaveV2 adaptor call builder | ||
| call := NewAaveV2ATokenAdaptorV2CallBuilder(common.HexToAddress("0x1")).DepositToAave(common.HexToAddress("0x2"), big.NewInt(100)).Build() | ||
|
|
||
| builder.FlashLoan([]common.Address{asset}, []*big.Int{big.NewInt(100)}, []*steward_proto.AdaptorCall{call}) | ||
|
|
||
| assert.Equal(t, 1, len(builder.calls)) | ||
| assert.IsType(t, &steward_proto.AaveV3DebtTokenAdaptorV1FlashLoan_FlashLoan{}, builder.calls[0].FlashLoan) | ||
|
|
||
| } |
There was a problem hiding this comment.
Consider adding more assertions to verify the correctness of the flash loan request.
Adding more assertions can help in verifying the correctness of the flash loan request.
assert.Equal(t, asset.Hex(), builder.calls[0].FlashLoan.Assets[0].Hex())
assert.Equal(t, big.NewInt(100), builder.calls[0].FlashLoan.Amounts[0])
assert.Equal(t, call, builder.calls[0].FlashLoan.AdaptorCalls[0])
go/builder/examples/adaptor_call.go
Outdated
| func ExampleAdaptorCall() { | ||
| // Get client and context | ||
| client, err := CreateTlsClient() | ||
| if err != nil { | ||
| panic(err) | ||
| } | ||
|
|
||
| ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) | ||
| defer cancel() | ||
|
|
||
| // Build adaptor call | ||
| adaptor := common.HexToAddress("0x1234567890000000000000000000000000000000") | ||
| token := common.HexToAddress("0x7Fc66500c84A76Ad7e9c93437bFc5Ac33E2DDaE9") | ||
| amount := big.NewInt(100000) | ||
| adaptorCall := adaptors.NewAaveV2ATokenAdaptorV2CallBuilder(adaptor). | ||
| DepositToAave(token, amount). | ||
| Build() | ||
|
|
||
| // Build call data | ||
| callData, err := builder.NewCellarCallDataBuilder(). | ||
| CallOnAdaptor(adaptorCall). | ||
| Build() | ||
| if err != nil { | ||
| panic(err) | ||
| } | ||
|
|
||
| // Build request | ||
| cellarId := common.HexToAddress("0x0") | ||
| request, err := builder.NewScheduleRequestBuilder(). | ||
| WithCellarID(cellarId). | ||
| WithChainID(1). | ||
| WithCallData(callData). | ||
| WithBlockHeight(100). | ||
| Build() | ||
| if err != nil { | ||
| panic(err) | ||
| } | ||
|
|
||
| // Send request | ||
| response, err := client.Schedule(ctx, request) | ||
| if err != nil { | ||
| panic(err) | ||
| } | ||
|
|
||
| fmt.Print(response) |
There was a problem hiding this comment.
Improve error handling by providing more context.
Providing more context in error messages can help in identifying the root cause of the issue.
client, err := CreateTlsClient()
if err != nil {
log.Fatalf("Error creating TLS client: %v", err)
}
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()
adaptor := common.HexToAddress("0x1234567890000000000000000000000000000000")
token := common.HexToAddress("0x7Fc66500c84A76Ad7e9c93437bFc5Ac33E2DDaE9")
amount := big.NewInt(100000)
adaptorCall := adaptors.NewAaveV2ATokenAdaptorV2CallBuilder(adaptor).
DepositToAave(token, amount).
Build()
callData, err := builder.NewCellarCallDataBuilder().
CallOnAdaptor(adaptorCall).
Build()
if err != nil {
log.Fatalf("Error building call data: %v", err)
}
cellarId := common.HexToAddress("0x0")
request, err := builder.NewScheduleRequestBuilder().
WithCellarID(cellarId).
WithChainID(1).
WithCallData(callData).
WithBlockHeight(100).
Build()
if err != nil {
log.Fatalf("Error building request: %v", err)
}
response, err := client.Schedule(ctx, request)
if err != nil {
log.Fatalf("Error sending request: %v", err)
}
| func ExampleBroadcastRequests() { | ||
| // Query the Sommelier chain to get the list of Steward instances | ||
| sommCtx := client.Context{}.WithNodeURI("http://localhost:26657") | ||
| sommQueryClient := pubsub.NewQueryClient(sommCtx) | ||
|
|
||
| res, err := sommQueryClient.QuerySubscribers(context.Background(), &pubsub.QuerySubscribersRequest{}) | ||
| if err != nil { | ||
| panic(err) | ||
| } | ||
|
|
||
| subscribers := res.Subscribers | ||
|
|
||
| // Get client | ||
| conn, client, err := CreateTlsClient() | ||
| if err != nil { | ||
| panic(err) | ||
| } | ||
|
|
||
| defer conn.Close() | ||
|
|
||
| // Build request | ||
| request, err := BuildMulticallRequest() | ||
| if err != nil { | ||
| panic(err) | ||
| } | ||
|
|
||
| // Send request to each subscriber | ||
| for _, subscriber := range subscribers { | ||
| response, err := client.Schedule(context.Background(), request) | ||
| if err != nil { | ||
| fmt.Printf("Error sending request to %s: %s\n", subscriber, err) | ||
| } | ||
|
|
||
| fmt.Print("Sent request to ", subscriber, " with response: ", response, "\n") | ||
| } |
There was a problem hiding this comment.
Consider sending requests concurrently for better performance.
The function is well-structured but could benefit from concurrent request sending for better performance.
// Send request to each subscriber concurrently
var wg sync.WaitGroup
for _, subscriber := range subscribers {
wg.Add(1)
go func(subscriber string) {
defer wg.Done()
response, err := client.Schedule(context.Background(), request)
if err != nil {
fmt.Printf("Error sending request to %s: %s\n", subscriber, err)
return
}
fmt.Print("Sent request to ", subscriber, " with response: ", response, "\n")
}(subscriber)
}
wg.Wait()
Summary by CodeRabbit
New Features
Examples
ExampleBroadcastRequests()that demonstrates how to send requests to multiple Steward instances.