Canto - DevABDee's results

Low Findings:

L1: Pool.CreatePool() doesn't checks if the pool already exists

The pool.CreatePool() function, as currently implemented, does not perform a check to see if the pool already exists. This means that multiple instances of the same pool can be created, potentially leading to duplication or inconsistencies within the system. Add a check for that.

func (k Keeper) CreatePool(ctx sdk.Context, counterpartyDenom string) types.Pool {
	poolID := types.GetPoolId(counterpartyDenom)

	// Check if the pool already exists
	if k.poolExists(ctx, poolID) {
		panic("Pool already exists") // or return an error, depending on your use case
	}

	sequence := k.getSequence(ctx)
	lptDenom := types.GetLptDenom(sequence)
	pool := &types.Pool{
		Id:                poolID,
		StandardDenom:     k.GetStandardDenom(ctx),
		CounterpartyDenom: counterpartyDenom,
		EscrowAddress:     types.GetReservePoolAddr(lptDenom).String(),
		LptDenom:          lptDenom,
	}
	k.setSequence(ctx, sequence+1)
	k.setPool(ctx, pool)
	return *pool
}

func (k Keeper) poolExists(ctx sdk.Context, poolID string) bool {
	// Implement the logic to check if the pool exists in the state or database
	// Return true if the pool exists, false otherwise
	// You need to customize this implementation based on your specific requirements
	_, found := k.getPool(ctx, poolID)
	return found
}

Note, the solution code is generated by ChatGPT :D

L2: Swap functions are missing transaction expiration checks

The swap.TradeInputForExactOutput() and swap.TradeExactInputForOutput() functions are part of a swap mechanism that allows users to exchange one type of token for another based on specific input and output requirements. However, it is important to note that these functions currently lack transaction expiration checks, which can introduce potential vulnerabilities.

To enhance the security and robustness of the swap mechanism, it is recommended to incorporate transaction expiration checks into the swap.TradeInputForExactOutput() and swap.TradeExactInputForOutput() functions. By doing so, the system can ensure that trades are executed within a valid timeframe, reducing the risk of potential exploits or discrepancies in token exchanges.

Non-Critical Findings:

NC-1: @param details/comments missing

func [OnRecvPacket()](https://github.com/code-423n4/2023-06-canto/blob/a4ff2fd2e67e77e36528fad99f9d88149a5e8532/Canto/x/onboarding/keeper/ibc_callbacks.go#L28) and some other functions as well are missing @param details/comments. Add that.

NC-2: package comments missing

The codebase lacks package comments, which are comments specifically written at the package level to provide an overview and description of the package's functionality and purpose. Package comments serve as a documentation header for the entire package and provide important context for developers and users of the package.

Including package comments is considered a good practice

NC-3: Improper function ordering

Define private functions first, followed by public functions. It's a common practice in go. That will increase the readability & audibility of the codebase.

NC-4: Line Limit Exceeds

Some code lines exceed the line limit (80). PoC: https://github.com/code-423n4/2023-06-canto/blob/a4ff2fd2e67e77e36528fad99f9d88149a5e8532/Canto/x/coinswap/keeper/swap.go#L146

NC-5: Explicitly set variables

It's good practice to set variables explicitly for better readability & audibility. Change from this:

var found bool

to this:

var found bool = false