Add generic order type test suite #86

zhongeric · 2023-01-17T22:18:50Z

Fix #83

test/base/BaseReactor.t.sol

test/reactors/DutchLimitOrderReactor.t.sol

marktoda

looks great! just a few thoughts

test/base/BaseReactor.t.sol

marktoda · 2023-01-19T00:08:46Z

test/base/BaseReactor.t.sol

+
+    MockERC20 tokenIn;
+    MockERC20 tokenOut;
+    MockFillContract fillContract;


just to note, and maybe this can be a different suite but we could test different fillContract strategies in a generic way as well. Would be nice to be sure that the matrix of [reactorType, fillContract] all works properly together

Yeah that would be cool, I'll look into that - what fillContracts do we have so far? Is it just DirectTaker and UniV3Executor?

but we def need to add a UR executor and maybe like order matching logic?

test/base/BaseReactor.t.sol

test/reactors/DutchLimitOrderReactor.t.sol

test/base/BaseReactor.t.sol

snreynolds · 2023-01-31T16:55:17Z

test/base/BaseReactor.t.sol

+        vm.expectEmit(false, false, false, true, address(reactor));
+        emit Fill(orderHash, address(this), maker, orderInfo.nonce);
+        // execute order
+        snapStart(string.concat(name(), "BaseExecuteSingle"));


sweet, love the customized name

mark's genius work

test/base/BaseReactor.t.sol

marktoda

looking good to me! agree w/ saras comments tho

marktoda · 2023-01-31T17:49:38Z

test/base/BaseReactor.t.sol

+
+    MockERC20 tokenIn;
+    MockERC20 tokenOut;
+    MockFillContract fillContract;


marktoda · 2023-01-31T17:50:02Z

test/base/BaseReactor.t.sol

+
+    MockERC20 tokenIn;
+    MockERC20 tokenOut;
+    MockFillContract fillContract;


but we def need to add a UR executor and maybe like order matching logic?

snreynolds

lgtm, not sure about @marktoda's comments though

snreynolds · 2023-02-02T17:39:49Z

test/reactors/DutchLimitOrderReactor.t.sol

 import {Test} from "forge-std/Test.sol";
+import "forge-std/console.sol";


can remove any console imports before merge

…er specific rn

…ments

… outputs

* continue Signed-off-by: Allen Lin <allenlin1992@hotmail.com> * continue Signed-off-by: Allen Lin <allenlin1992@hotmail.com> * add snapshot Signed-off-by: Allen Lin <allenlin1992@hotmail.com> * continue Signed-off-by: Allen Lin <allenlin1992@hotmail.com> * write testTwoOrders Signed-off-by: Allen Lin <allenlin1992@hotmail.com> * continue Signed-off-by: Allen Lin <allenlin1992@hotmail.com> * comment Signed-off-by: Allen Lin <allenlin1992@hotmail.com> * write `testFillerHasInsufficientOutput` Signed-off-by: Allen Lin <allenlin1992@hotmail.com> * fix merge main Signed-off-by: Allen Lin <allenlin1992@hotmail.com> * write testSingleOrderWithFee Signed-off-by: Allen Lin <allenlin1992@hotmail.com> * write testThreeOrdersWithFees() Signed-off-by: Allen Lin <allenlin1992@hotmail.com> * remove code duplication Signed-off-by: Allen Lin <allenlin1992@hotmail.com> * make permit2 an address that needs to be casted to appropriate interface Signed-off-by: Allen Lin <allenlin1992@hotmail.com> * make DIRECT_TAKER_FILL constant Signed-off-by: Allen Lin <allenlin1992@hotmail.com> * write testFillerLacksApproval() Signed-off-by: Allen Lin <allenlin1992@hotmail.com> * make DIRECT_TAKER_FILL and internal variable Signed-off-by: Allen Lin <allenlin1992@hotmail.com> * use SafeCast.toUint160 Signed-off-by: Allen Lin <allenlin1992@hotmail.com> * delete DirectTakerExecutor.t.sol Signed-off-by: Allen Lin <allenlin1992@hotmail.com> --------- Signed-off-by: Allen Lin <allenlin1992@hotmail.com>

* Add test for NonceReuse to DLO reactor test * Ok have something, testExecute in baseTest passes, but only limit order specific rn * remove comment * Change to createAndSignOrder and use abiEncodedOrder in base tests * Add signature replay base test * Add inputAmount, outputAmount to createAndSignOrder * Add base prefix to BaseReactorTest * Remove extra createReactor call * Add test for BaseExecuteBatch, and createAndSignBatchOrders * Clean up and add comments * Refactor createSginOrder to return SignedOrder and other resolved comments * Remove virtual from base tests * Allow specification of OrderInfo in generic func * compiles! * Update gas snaps * Add NonceReuse base test * Add comments * Add comments * Remove nonceReuse from dutchLimit * forge fmt * Revisions, and add ArrayBuilder to create 1d and 2d arays easier * Add support in LimitOrderReactor.t.sol for batch orders with multiple outputs * Direct taker macro (#77) * continue Signed-off-by: Allen Lin <allenlin1992@hotmail.com> * continue Signed-off-by: Allen Lin <allenlin1992@hotmail.com> * add snapshot Signed-off-by: Allen Lin <allenlin1992@hotmail.com> * continue Signed-off-by: Allen Lin <allenlin1992@hotmail.com> * write testTwoOrders Signed-off-by: Allen Lin <allenlin1992@hotmail.com> * continue Signed-off-by: Allen Lin <allenlin1992@hotmail.com> * comment Signed-off-by: Allen Lin <allenlin1992@hotmail.com> * write `testFillerHasInsufficientOutput` Signed-off-by: Allen Lin <allenlin1992@hotmail.com> * fix merge main Signed-off-by: Allen Lin <allenlin1992@hotmail.com> * write testSingleOrderWithFee Signed-off-by: Allen Lin <allenlin1992@hotmail.com> * write testThreeOrdersWithFees() Signed-off-by: Allen Lin <allenlin1992@hotmail.com> * remove code duplication Signed-off-by: Allen Lin <allenlin1992@hotmail.com> * make permit2 an address that needs to be casted to appropriate interface Signed-off-by: Allen Lin <allenlin1992@hotmail.com> * make DIRECT_TAKER_FILL constant Signed-off-by: Allen Lin <allenlin1992@hotmail.com> * write testFillerLacksApproval() Signed-off-by: Allen Lin <allenlin1992@hotmail.com> * make DIRECT_TAKER_FILL and internal variable Signed-off-by: Allen Lin <allenlin1992@hotmail.com> * use SafeCast.toUint160 Signed-off-by: Allen Lin <allenlin1992@hotmail.com> * delete DirectTakerExecutor.t.sol Signed-off-by: Allen Lin <allenlin1992@hotmail.com> --------- Signed-off-by: Allen Lin <allenlin1992@hotmail.com> * udpate gas snap * Remove redundant testExecutebatch * remove old gas snap * change func in lib type to internal * remove unused initNested * Remove console import and sign * add .gitignore --------- Signed-off-by: Allen Lin <allenlin1992@hotmail.com> Co-authored-by: AzFlin <allenlin1992@hotmail.com>

zhongeric changed the title ~~[WIP] Add test for NonceReuse to DLO reactor test~~ [WIP] Add generic order type test suite #83 Jan 17, 2023

zhongeric changed the title ~~[WIP] Add generic order type test suite #83~~ [WIP] Add generic order type test suite Jan 17, 2023

zhongeric commented Jan 18, 2023

View reviewed changes

test/base/BaseReactor.t.sol Show resolved Hide resolved

zhongeric commented Jan 18, 2023

View reviewed changes

test/base/BaseReactor.t.sol Outdated Show resolved Hide resolved

zhongeric commented Jan 18, 2023

View reviewed changes

test/base/BaseReactor.t.sol Outdated Show resolved Hide resolved

zhongeric commented Jan 18, 2023

View reviewed changes

test/reactors/DutchLimitOrderReactor.t.sol Outdated Show resolved Hide resolved

marktoda reviewed Jan 19, 2023

View reviewed changes

test/reactors/DutchLimitOrderReactor.t.sol Outdated Show resolved Hide resolved

zhongeric changed the title ~~[WIP] Add generic order type test suite~~ Add generic order type test suite Jan 30, 2023

zhongeric marked this pull request as ready for review January 30, 2023 21:26

zhongeric requested review from marktoda, snreynolds and azflin January 30, 2023 21:32

snreynolds requested changes Jan 31, 2023

View reviewed changes

marktoda reviewed Jan 31, 2023

View reviewed changes

zhongeric requested a review from snreynolds January 31, 2023 21:58

snreynolds approved these changes Feb 2, 2023

View reviewed changes

marktoda approved these changes Feb 2, 2023

View reviewed changes

zhongeric added 12 commits February 2, 2023 13:27

Add test for NonceReuse to DLO reactor test

737720e

Ok have something, testExecute in baseTest passes, but only limit ord…

11cf4e6

…er specific rn

remove comment

b2316da

Change to createAndSignOrder and use abiEncodedOrder in base tests

6870db5

Add signature replay base test

a01d8fd

Add inputAmount, outputAmount to createAndSignOrder

5195ef6

Add base prefix to BaseReactorTest

b0ab3ad

Remove extra createReactor call

796d6ac

Add test for BaseExecuteBatch, and createAndSignBatchOrders

4d6cdff

Clean up and add comments

a0e8002

Refactor createSginOrder to return SignedOrder and other resolved com…

355e501

…ments

Remove virtual from base tests

a4bc05e

zhongeric and others added 18 commits February 2, 2023 13:27

Allow specification of OrderInfo in generic func

576184f

compiles!

b93f884

Update gas snaps

b259765

Add NonceReuse base test

365d8b2

Add comments

69ffefa

Add comments

8b0f874

Remove nonceReuse from dutchLimit

1890916

forge fmt

3a69035

Revisions, and add ArrayBuilder to create 1d and 2d arays easier

6e25f21

Add support in LimitOrderReactor.t.sol for batch orders with multiple…

144b651

… outputs

udpate gas snap

cf13bcd

Remove redundant testExecutebatch

31040d4

remove old gas snap

e61c2e5

change func in lib type to internal

87feb00

remove unused initNested

2cf1b98

Remove console import and sign

9553e82

add .gitignore

c2058a2

zhongeric force-pushed the generic-test-suite branch from bc6cc3b to c2058a2 Compare February 2, 2023 18:29

Merge branch 'main' into generic-test-suite

f8dc1e0

zhongeric merged commit 9b6b759 into main Feb 2, 2023

zhongeric mentioned this pull request Feb 3, 2023

Add capability to test full fillContract, executor matrix #93

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add generic order type test suite #86

Add generic order type test suite #86

zhongeric commented Jan 17, 2023 •

edited

marktoda left a comment

marktoda Jan 19, 2023

zhongeric Jan 19, 2023

marktoda Jan 31, 2023

marktoda Jan 31, 2023

snreynolds Jan 31, 2023

zhongeric Jan 31, 2023

marktoda left a comment

marktoda Jan 31, 2023

marktoda Jan 31, 2023

snreynolds left a comment

snreynolds Feb 2, 2023

		import {Test} from "forge-std/Test.sol";
		import "forge-std/console.sol";

Add generic order type test suite #86

Add generic order type test suite #86

Conversation

zhongeric commented Jan 17, 2023 • edited

marktoda left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marktoda left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

snreynolds left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhongeric commented Jan 17, 2023 •

edited