From a3643e01e3c2052453dc3a7dd165a4dd4d9a3cad Mon Sep 17 00:00:00 2001 From: codinghistorian Date: Tue, 23 Sep 2025 15:26:29 +0400 Subject: [PATCH 1/2] refactor: reorder pool transfers to follow CEI pattern Reorder internal state updates before cross-contract calls in: - safeTransfer: update balances before SafeGRC20Transfer - safeTransferFrom: update balances before SafeGRC20TransferFrom - collectProtocol: update balances before token transfers Applies checks-effects-interactions pattern for better security practices. --- contract/r/gnoswap/v1/pool/pool.gno | 37 ++++---- contract/r/gnoswap/v1/pool/transfer.gno | 117 ++++++++++++------------ 2 files changed, 80 insertions(+), 74 deletions(-) diff --git a/contract/r/gnoswap/v1/pool/pool.gno b/contract/r/gnoswap/v1/pool/pool.gno index 8759ac4fb..ced97440b 100644 --- a/contract/r/gnoswap/v1/pool/pool.gno +++ b/contract/r/gnoswap/v1/pool/pool.gno @@ -327,23 +327,26 @@ func collectProtocol( amount0 := u256Min(amount0Req, pool.ProtocolFeesToken0()) amount1 := u256Min(amount1Req, pool.ProtocolFeesToken1()) - amount0, amount1 = pool.saveProtocolFees(amount0.Clone(), amount1.Clone()) - uAmount0 := safeConvertToInt64(amount0) - uAmount1 := safeConvertToInt64(amount1) - - common.SafeGRC20Transfer(cross, pool.token0Path, recipient, uAmount0) - newBalanceToken0, err := updatePoolBalance(pool.BalanceToken0(), pool.BalanceToken1(), amount0, true) - if err != nil { - panic(err) - } - pool.balances.token0 = newBalanceToken0 - - common.SafeGRC20Transfer(cross, pool.token1Path, recipient, uAmount1) - newBalanceToken1, err := updatePoolBalance(pool.BalanceToken0(), pool.BalanceToken1(), amount1, false) - if err != nil { - panic(err) - } - pool.balances.token1 = newBalanceToken1 + amount0, amount1 = pool.saveProtocolFees(amount0.Clone(), amount1.Clone()) + + // Effects: update internal pool balances first + newBalanceToken0, err := updatePoolBalance(pool.BalanceToken0(), pool.BalanceToken1(), amount0, true) + if err != nil { + panic(err) + } + pool.balances.token0 = newBalanceToken0 + + newBalanceToken1, err := updatePoolBalance(pool.BalanceToken0(), pool.BalanceToken1(), amount1, false) + if err != nil { + panic(err) + } + pool.balances.token1 = newBalanceToken1 + + // Interactions: then perform external transfers + uAmount0 := safeConvertToInt64(amount0) + uAmount1 := safeConvertToInt64(amount1) + common.SafeGRC20Transfer(cross, pool.token0Path, recipient, uAmount0) + common.SafeGRC20Transfer(cross, pool.token1Path, recipient, uAmount1) return amount0.ToString(), amount1.ToString() } diff --git a/contract/r/gnoswap/v1/pool/transfer.gno b/contract/r/gnoswap/v1/pool/transfer.gno index 05f4f2497..468e98d2c 100644 --- a/contract/r/gnoswap/v1/pool/transfer.gno +++ b/contract/r/gnoswap/v1/pool/transfer.gno @@ -45,28 +45,29 @@ func (p *Pool) safeTransfer( )) } - absAmount := amount.Abs() - - token0 := p.BalanceToken0() - token1 := p.BalanceToken1() - - if err := validatePoolBalance(token0, token1, absAmount, isToken0); err != nil { - panic(err) - } - amountInt64 := safeConvertToInt64(absAmount) - - common.SafeGRC20Transfer(cross, tokenPath, to, amountInt64) - - newBalance, err := updatePoolBalance(token0, token1, absAmount, isToken0) - if err != nil { - panic(err) - } - - if isToken0 { - p.balances.token0 = newBalance - } else { - p.balances.token1 = newBalance - } + absAmount := amount.Abs() + + token0 := p.BalanceToken0() + token1 := p.BalanceToken1() + + if err := validatePoolBalance(token0, token1, absAmount, isToken0); err != nil { + panic(err) + } + + // Effects: compute and apply new internal balance first + newBalance, err := updatePoolBalance(token0, token1, absAmount, isToken0) + if err != nil { + panic(err) + } + if isToken0 { + p.balances.token0 = newBalance + } else { + p.balances.token1 = newBalance + } + + // Interactions: perform external transfer after state change + amountInt64 := safeConvertToInt64(absAmount) + common.SafeGRC20Transfer(cross, tokenPath, to, amountInt64) } // safeTransferFrom securely transfers tokens into the pool while ensuring balance consistency. @@ -107,41 +108,43 @@ func (p *Pool) safeTransferFrom( amount *u256.Uint, isToken0 bool, ) { - amountInt64 := safeConvertToInt64(amount) - - token := common.GetToken(tokenPath) - beforeBalance := token.BalanceOf(to) - - common.SafeGRC20TransferFrom(cross, tokenPath, from, to, amountInt64) - - afterBalance := token.BalanceOf(to) - if (beforeBalance + amountInt64) != afterBalance { - panic(ufmt.Sprintf( - "%v. beforeBalance(%d) + amount(%d) != afterBalance(%d)", - errTransferFailed, beforeBalance, amountInt64, afterBalance, - )) - } - - // update pool balances - if isToken0 { - beforeToken0 := p.balances.token0.Clone() - p.balances.token0 = u256.Zero().Add(p.balances.token0, amount) - if p.balances.token0.Lt(beforeToken0) { - panic(ufmt.Sprintf( - "%v. token0(%s) < beforeToken0(%s)", - errBalanceUpdateFailed, p.balances.token0.ToString(), beforeToken0.ToString(), - )) - } - } else { - beforeToken1 := p.balances.token1.Clone() - p.balances.token1 = u256.Zero().Add(p.balances.token1, amount) - if p.balances.token1.Lt(beforeToken1) { - panic(ufmt.Sprintf( - "%v. token1(%s) < beforeToken1(%s)", - errBalanceUpdateFailed, p.balances.token1.ToString(), beforeToken1.ToString(), - )) - } - } + amountInt64 := safeConvertToInt64(amount) + + token := common.GetToken(tokenPath) + beforeBalance := token.BalanceOf(to) + + // Effects: update internal balances before interaction + if isToken0 { + beforeToken0 := p.balances.token0.Clone() + p.balances.token0 = u256.Zero().Add(p.balances.token0, amount) + if p.balances.token0.Lt(beforeToken0) { + panic(ufmt.Sprintf( + "%v. token0(%s) < beforeToken0(%s)", + errBalanceUpdateFailed, p.balances.token0.ToString(), beforeToken0.ToString(), + )) + } + } else { + beforeToken1 := p.balances.token1.Clone() + p.balances.token1 = u256.Zero().Add(p.balances.token1, amount) + if p.balances.token1.Lt(beforeToken1) { + panic(ufmt.Sprintf( + "%v. token1(%s) < beforeToken1(%s)", + errBalanceUpdateFailed, p.balances.token1.ToString(), beforeToken1.ToString(), + )) + } + } + + // Interactions: perform external transfer after state change + common.SafeGRC20TransferFrom(cross, tokenPath, from, to, amountInt64) + + // Post-condition: verify external balance reflects the transfer + afterBalance := token.BalanceOf(to) + if (beforeBalance + amountInt64) != afterBalance { + panic(ufmt.Sprintf( + "%v. beforeBalance(%d) + amount(%d) != afterBalance(%d)", + errTransferFailed, beforeBalance, amountInt64, afterBalance, + )) + } } // validatePoolBalance checks if the pool has sufficient balance of either token0 and token1 From e4feeb9c95ab45657ff229bfdc677011253352d6 Mon Sep 17 00:00:00 2001 From: codinghistorian Date: Tue, 23 Sep 2025 15:40:33 +0400 Subject: [PATCH 2/2] ci(tlin): make tlin_check fork-PR compatible (checkout PR head) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use actions/checkout with PR head repo/ref and fetch-depth: 0 repository: ${{ github.event.pull_request.head.repo.full_name }} ref: ${{ github.event.pull_request.head.ref }} fetch-depth: 0 Fixes checkout failure when the branch doesn’t exist in the base repo (forked PRs) No behavior changes to the job; improves CI reliability for external contributors --- .github/workflows/tlin_check.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/tlin_check.yml b/.github/workflows/tlin_check.yml index a66ec53d9..2221e77db 100644 --- a/.github/workflows/tlin_check.yml +++ b/.github/workflows/tlin_check.yml @@ -16,7 +16,9 @@ jobs: - name: checkout code uses: actions/checkout@v4 with: + repository: ${{ github.event.pull_request.head.repo.full_name }} ref: ${{ github.event.pull_request.head.ref }} + fetch-depth: 0 - name: checkout tlin uses: actions/checkout@v4