@ysv reviewed 3 files and all commit messages, and made 3 comments.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on masihyeganeh, metalarm10, miladz68, and TxCorpi0x).

integration-tests/modules/dex_test.go line 3058 at r1 (raw file):

// via raw bank keeper. This test asserts the fix: the second order (which would
// lock frozen tokens) must be rejected.
func TestFrozenTokenEscapeViaDEX(t *testing.T) {

are these 2 tests from immunify bugreport ?

they are almost equal so I would keep 1 one of them. Either integration or keeper test

x/asset/ft/keeper/keeper_dex.go line 69 at r1 (raw file):

	for _, send := range actions.Send {
		// Validate frozen balances before sending to prevent transferring frozen tokens

does this part of code run during order execution ?

As far as I remember our plan was to make sure that if order is inside orderbook it will be guaranteed executable when matched. And I think this check brakes such logic

Lets discuss

integration-tests/modules/dex_test.go line 3230 at r1 (raw file):

		placeOrder2,
	)
	requireT.Error(err, "Second order of 400,000 must fail: it would lock frozen tokens; available spendable is 0")

maybe it makes sense to check that address is not able to lock 100k or 10k also.
Smaller portion of frozen/locked.

@masihyeganeh made 3 comments.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on metalarm10, miladz68, TxCorpi0x, and ysv).

integration-tests/modules/dex_test.go line 3058 at r1 (raw file):

Yes. kept the integration test only

integration-tests/modules/dex_test.go line 3230 at r1 (raw file):

Done.

x/asset/ft/keeper/keeper_dex.go line 69 at r1 (raw file):

It was an extra safeguard for a defense in depth. I removed it and still both tests are passing. I also checked the original test and it failed

@miladz68 made 1 comment.
Reviewable status: 0 of 3 files reviewed, 4 unresolved discussions (waiting on masihyeganeh, metalarm10, TxCorpi0x, and ysv).

x/asset/ft/keeper/keeper_dex.go line 670 at r2 (raw file):

	// validate that we don't use frozen coins (if freezing feature is enabled)
	def, err := k.getDefinitionOrNil(ctx, coin.Denom)

Do I understand correctly that we check that frozen is not violated after a dex match happens ?

@ysv made 2 comments and resolved 3 discussions.
Reviewable status: 0 of 3 files reviewed, 3 unresolved discussions (waiting on masihyeganeh, metalarm10, miladz68, and TxCorpi0x).

x/asset/ft/keeper/keeper_dex.go line 670 at r2 (raw file):

	// validate that we don't use frozen coins (if freezing feature is enabled)
	def, err := k.getDefinitionOrNil(ctx, coin.Denom)

@miladz68 @masihyeganeh
lets double check and make sure this is called ONLY on order placement step not inside matching

integration-tests/modules/dex_test.go line 3058 at r2 (raw file):

// via raw bank keeper. This test asserts the fix: the second order (which would
// lock frozen tokens) must be rejected.
func TestFrozenTokenEscapeViaDEX(t *testing.T) {

lets also add intgr or unit-test for the following logic:

1. user has 100TKN availble
2. user creates order for 80TKN sell
3. issuer freezes 60/100 TKN
4. DEX order matched
5. Order should still be executed

(number are subject to change)

@masihyeganeh made 1 comment.
Reviewable status: 0 of 4 files reviewed, 3 unresolved discussions (waiting on metalarm10, miladz68, TxCorpi0x, and ysv).

integration-tests/modules/dex_test.go line 3058 at r2 (raw file):

Done.

@miladz68 made 1 comment.
Reviewable status: 0 of 5 files reviewed, 4 unresolved discussions (waiting on masihyeganeh, metalarm10, TxCorpi0x, and ysv).

x/asset/ft/keeper/keeper.go line 1017 at r3 (raw file):

	}

	// Spendable = balance - dexLocked - bankLocked - frozen (additive when freezing enabled)

should we check the spendable balance query ?

@ysv reviewed 5 files and all commit messages, made 1 comment, and resolved 2 discussions.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on masihyeganeh, metalarm10, and TxCorpi0x).

integration-tests/modules/dex_test.go line 3306 at r4 (raw file):

	requireT.NoError(err)

	// Setup: token with freezing, Alice has 100 TKN (1,000,000), Bob has quote

nit: maybe to match with comments we operate with millions of tokens

100 TKN = 100_000_000
80 TKN = 80_000_000

just easier to read

@miladz68 reviewed all commit messages and resolved 1 discussion.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on masihyeganeh, metalarm10, and TxCorpi0x).

@TxCorpi0x partially reviewed 5 files, made 2 comments, and resolved 1 discussion.
Reviewable status: 3 of 6 files reviewed, 1 unresolved discussion (waiting on metalarm10, miladz68, and ysv).

integration-tests/modules/dex_test.go line 3306 at r4 (raw file):

Done

x/asset/ft/keeper/keeper.go line 1017 at r3 (raw file):

Done

@ysv reviewed 3 files and all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on metalarm10 and miladz68).

@metalarm10 reviewed 6 files and all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on miladz68).