feat(sai-trading): Trades against testnet network

[expertdicer]

Abstract

• Closes #AAA or https://github.../issue/AAA

[gemini-code-assist]

Code Review

This pull request introduces a comprehensive CLI tool and an automated trading bot for the Sai trading platform. The changes are well-structured, with clear separation of concerns between the CLI, the core trader service, and configuration management. The addition of configuration files (networks.toml, auto-trader.json) and justfile recipes significantly improves usability.

My review focuses on the new Go code. I've identified a critical bug in how trade parameters are prepared, issues with the --long flag in the CLI, and incorrect logic for placing stop orders in the auto-trader. The feedback includes specific code suggestions to address these points. Overall, this is a great addition, and with these fixes, it will be a robust tool for interacting with the trading platform.

I am having trouble creating individual review comments. Click here to see my feedback.

sai-trading/services/evmtrader/trade_params.go (36-37)

The trade amount is incorrectly determined. It directly uses t.cfg.TradeSize, which is only set by the --trade-size flag and defaults to 0. This ignores the --trade-size-min and --trade-size-max flags, and also skips the crucial balance check.

The function determineTradeAmount, which contains the correct logic for calculating trade size and checking balance, is defined but never called within prepareTradeFromConfig. This is a critical bug that will cause trades from the open command to fail or use an incorrect size.

The TODO on line 36 indicates this was likely unfinished. You should replace the current implementation with a call to determineTradeAmount.

	// Step 2: Determine trade amount (validates balance internally)
	tradeAmt, err := t.determineTradeAmount(balance)
	if err != nil {
		return nil, err
	}
	if tradeAmt == nil {
		// This means insufficient balance or no trade size configured.
		// The log is already handled in determineTradeAmount.
		return nil, nil
	}

sai-trading/cmd/trader/main.go (114)

The default value for the --long flag is set to false, but the help text says (default: true). This is contradictory and confusing. The effective behavior when the flag is omitted is indeed long (true), due to how a nil pointer is handled by the evmtrader service. To make the flag's behavior explicit and align with the documentation, the default value should be set to true.

cmd.Flags().BoolVar(&long, "long", true, "Trade direction: true for long, false for short")

sai-trading/services/evmtrader/auto_trader.go (197-217)

The logic for setting the trigger price for stop orders in the auto-trader appears to be incorrect for creating stop-entry orders.

• For a long stop order, the trigger price is set below the market price. A buy-stop order should be placed above the market price to enter a trade on an upward breakout.
• For a short stop order, the trigger price is set above the market price. A sell-stop order should be placed below the market price to enter a trade on a downward breakdown.

The current implementation effectively sets stop-loss orders for positions that don't exist yet, which will likely not execute as intended for opening new trades. The comments are also confusing as they refer to "stop loss".

if isLong {
					// For long limit orders, set trigger price below market (buy cheaper)
					if tradeType == TradeTypeLimit {
						triggerPrice := marketPrice * (1.0 - adjustmentPercent)
						openPrice = &triggerPrice
					} else { // stop
						// For long stop-entry orders, set trigger price above market (buy on breakout)
						triggerPrice := marketPrice * (1.0 + adjustmentPercent)
						openPrice = &triggerPrice
					}
				} else {
					// For short limit orders, set trigger price above market (sell higher)
					if tradeType == TradeTypeLimit {
						triggerPrice := marketPrice * (1.0 + adjustmentPercent)
						openPrice = &triggerPrice
					} else { // stop
						// For short stop-entry orders, set trigger price below market (sell on breakdown)
						triggerPrice := marketPrice * (1.0 - adjustmentPercent)
						openPrice = &triggerPrice
					}
				}

sai-trading/cmd/trader/main.go (98-108)

The logic for handling the --long flag by checking cmd.Flags().Changed("long") is overly complex. Assuming the flag's default value is corrected to true (as suggested in a separate comment), you can simplify this RunE function by removing the conditional pointer logic for long and always passing its address to runOpen.

RunE: func(cmd *cobra.Command, args []string) error {
			var openPricePtr *float64
			if cmd.Flags().Changed("open-price") {
				openPricePtr = &openPrice
			}
			return runOpen(tradeSize, leverage, &long, marketIndex, collateralIndex, tradeType, openPricePtr, tradeJSONFile)
		},