From eaef4ce924d66b468843443c00641b8567fe226f Mon Sep 17 00:00:00 2001 From: boolafish Date: Wed, 4 Mar 2020 16:57:22 +0900 Subject: [PATCH 1/8] test: integ test for challenge IFE input spend for all output indexes This is an effort to add tests to cover challenge IFE input spent on all output indexes. We found a bug that we are using wrong indexes in contract code. But, this test end up does not break the existing contract as although it is attracting wrong output, we are only using the output type with that output. With current system, we only have 1 output type for payment tx. As a result it failed to break with this test.. https://github.com/omisego/security-issues/issues/15 --- ...st_challenge_in_flight_exit_input_spent.py | 182 +++++++++++------- 1 file changed, 115 insertions(+), 67 deletions(-) diff --git a/plasma_framework/python_tests/tests/contracts/root_chain/test_challenge_in_flight_exit_input_spent.py b/plasma_framework/python_tests/tests/contracts/root_chain/test_challenge_in_flight_exit_input_spent.py index bd5e2480a..ac3e067bd 100644 --- a/plasma_framework/python_tests/tests/contracts/root_chain/test_challenge_in_flight_exit_input_spent.py +++ b/plasma_framework/python_tests/tests/contracts/root_chain/test_challenge_in_flight_exit_input_spent.py @@ -1,71 +1,119 @@ import pytest from eth_tester.exceptions import TransactionFailed from plasma_core.constants import NULL_ADDRESS +from plasma_core.utils.transactions import decode_utxo_id, encode_utxo_id + + +# # should succeed even when phase 2 of in-flight exit is over +# @pytest.mark.parametrize("period", [1, 2, 3, 4]) +# def test_challenge_in_flight_exit_input_spent_should_succeed_in_all_periods(testlang, period): +# owner_1, owner_2, amount = testlang.accounts[0], testlang.accounts[1], 100 +# deposit_id = testlang.deposit(owner_1, amount) +# spend_id = testlang.spend_utxo([deposit_id], [owner_1], [(owner_1.address, NULL_ADDRESS, 100)]) +# double_spend_id = testlang.spend_utxo([deposit_id], [owner_1], [(owner_2.address, NULL_ADDRESS, 100)], force_invalid=True) +# testlang.start_in_flight_exit(spend_id) +# testlang.piggyback_in_flight_exit_input(spend_id, 0, owner_1) +# testlang.forward_to_period(period) + +# testlang.challenge_in_flight_exit_input_spent(spend_id, double_spend_id, owner_2) + +# in_flight_exit = testlang.get_in_flight_exit(spend_id) +# assert not in_flight_exit.input_piggybacked(0) + + + +@pytest.mark.parametrize("num_outputs", [1, 2, 3, 4]) +def test_challenge_in_flight_exit_should_succeed_for_all_output_indexes(testlang, num_outputs): + deposit_amount = 100 + deposit_id = testlang.deposit(testlang.accounts[0], deposit_amount) + + owners = [] + outputs = [] + tx_output_amount = deposit_amount // num_outputs + for i in range(0, num_outputs): + owners.append(testlang.accounts[i]) + outputs.append((testlang.accounts[i].address, NULL_ADDRESS, tx_output_amount)) + + input_tx_id = testlang.spend_utxo([deposit_id], owners, outputs=outputs) + + blknum, tx_index, _ = decode_utxo_id(input_tx_id) + double_spend_output_index = num_outputs - 1 + double_spend_owner = owners[double_spend_output_index] + double_spend_utxo = encode_utxo_id(blknum, tx_index, double_spend_output_index) + + ife_output_amount = tx_output_amount // 2 + ife_tx_id = testlang.spend_utxo( + [double_spend_utxo], + [double_spend_owner], + [(owners[0].address, NULL_ADDRESS, ife_output_amount)] + ) + + challenge_tx_id = testlang.spend_utxo( + [double_spend_utxo], + [double_spend_owner], + [ + (owners[0].address, NULL_ADDRESS, ife_output_amount), + (owners[0].address, NULL_ADDRESS, ife_output_amount) + ], + force_invalid=True + ) + + testlang.start_in_flight_exit(ife_tx_id) + testlang.piggyback_in_flight_exit_input(ife_tx_id, 0, double_spend_owner) + testlang.forward_to_period(1) + testlang.challenge_in_flight_exit_input_spent(ife_tx_id, challenge_tx_id, double_spend_owner) + + in_flight_exit = testlang.get_in_flight_exit(ife_tx_id) + assert not in_flight_exit.input_piggybacked(double_spend_output_index) + + +# def test_challenge_in_flight_exit_input_spent_not_piggybacked_should_fail(testlang): +# owner_1, owner_2, amount = testlang.accounts[0], testlang.accounts[1], 100 +# deposit_id = testlang.deposit(owner_1, amount) +# spend_id = testlang.spend_utxo([deposit_id], [owner_1], [(owner_1.address, NULL_ADDRESS, 100)]) +# double_spend_id = testlang.spend_utxo([deposit_id], [owner_1], [(owner_2.address, NULL_ADDRESS, 100)], force_invalid=True) +# testlang.start_in_flight_exit(spend_id) +# testlang.forward_to_period(2) + +# with pytest.raises(TransactionFailed): +# testlang.challenge_in_flight_exit_input_spent(spend_id, double_spend_id, owner_2) + + +# def test_challenge_in_flight_exit_input_spent_same_tx_should_fail(testlang): +# owner_1, owner_2, amount = testlang.accounts[0], testlang.accounts[1], 100 +# deposit_id = testlang.deposit(owner_1, amount) +# spend_id = testlang.spend_utxo([deposit_id], [owner_1], [(owner_1.address, NULL_ADDRESS, 100)]) +# testlang.start_in_flight_exit(spend_id) +# testlang.piggyback_in_flight_exit_input(spend_id, 0, owner_1) +# testlang.forward_to_period(2) + +# with pytest.raises(TransactionFailed): +# testlang.challenge_in_flight_exit_input_spent(spend_id, spend_id, owner_2) + + +# def test_challenge_in_flight_exit_input_spent_unrelated_tx_should_fail(testlang): +# owner_1, owner_2, amount = testlang.accounts[0], testlang.accounts[1], 100 +# deposit_id_1 = testlang.deposit(owner_1, amount) +# deposit_id_2 = testlang.deposit(owner_1, amount) +# spend_id = testlang.spend_utxo([deposit_id_1], [owner_1], [(owner_1.address, NULL_ADDRESS, 100)]) +# unrelated_spend_id = testlang.spend_utxo([deposit_id_2], [owner_1], [(owner_1.address, NULL_ADDRESS, 100)]) +# testlang.start_in_flight_exit(spend_id) +# testlang.piggyback_in_flight_exit_input(spend_id, 0, owner_1) +# testlang.forward_to_period(2) + +# with pytest.raises(TransactionFailed): +# testlang.challenge_in_flight_exit_input_spent(spend_id, unrelated_spend_id, owner_2) + + +# def test_challenge_in_flight_exit_input_spent_invalid_signature_should_fail(testlang): +# owner_1, owner_2, amount = testlang.accounts[0], testlang.accounts[1], 100 +# deposit_id = testlang.deposit(owner_1, amount) +# spend_id = testlang.spend_utxo([deposit_id], [owner_1], [(owner_1.address, NULL_ADDRESS, 100)]) +# double_spend_id = testlang.spend_utxo([deposit_id], [owner_2], [(owner_2.address, NULL_ADDRESS, 100)], force_invalid=True) +# testlang.start_in_flight_exit(spend_id) +# testlang.piggyback_in_flight_exit_input(spend_id, 0, owner_1) +# testlang.forward_to_period(2) + +# with pytest.raises(TransactionFailed): +# testlang.challenge_in_flight_exit_input_spent(spend_id, double_spend_id, owner_2) - -# should succeed even when phase 2 of in-flight exit is over -@pytest.mark.parametrize("period", [1, 2, 4]) -def test_challenge_in_flight_exit_input_spent_should_succeed(testlang, period): - owner_1, owner_2, amount = testlang.accounts[0], testlang.accounts[1], 100 - deposit_id = testlang.deposit(owner_1, amount) - spend_id = testlang.spend_utxo([deposit_id], [owner_1], [(owner_1.address, NULL_ADDRESS, 100)]) - double_spend_id = testlang.spend_utxo([deposit_id], [owner_1], [(owner_2.address, NULL_ADDRESS, 100)], force_invalid=True) - testlang.start_in_flight_exit(spend_id) - testlang.piggyback_in_flight_exit_input(spend_id, 0, owner_1) - testlang.forward_to_period(period) - - testlang.challenge_in_flight_exit_input_spent(spend_id, double_spend_id, owner_2) - - in_flight_exit = testlang.get_in_flight_exit(spend_id) - assert not in_flight_exit.input_piggybacked(0) - - -def test_challenge_in_flight_exit_input_spent_not_piggybacked_should_fail(testlang): - owner_1, owner_2, amount = testlang.accounts[0], testlang.accounts[1], 100 - deposit_id = testlang.deposit(owner_1, amount) - spend_id = testlang.spend_utxo([deposit_id], [owner_1], [(owner_1.address, NULL_ADDRESS, 100)]) - double_spend_id = testlang.spend_utxo([deposit_id], [owner_1], [(owner_2.address, NULL_ADDRESS, 100)], force_invalid=True) - testlang.start_in_flight_exit(spend_id) - testlang.forward_to_period(2) - - with pytest.raises(TransactionFailed): - testlang.challenge_in_flight_exit_input_spent(spend_id, double_spend_id, owner_2) - - -def test_challenge_in_flight_exit_input_spent_same_tx_should_fail(testlang): - owner_1, owner_2, amount = testlang.accounts[0], testlang.accounts[1], 100 - deposit_id = testlang.deposit(owner_1, amount) - spend_id = testlang.spend_utxo([deposit_id], [owner_1], [(owner_1.address, NULL_ADDRESS, 100)]) - testlang.start_in_flight_exit(spend_id) - testlang.piggyback_in_flight_exit_input(spend_id, 0, owner_1) - testlang.forward_to_period(2) - - with pytest.raises(TransactionFailed): - testlang.challenge_in_flight_exit_input_spent(spend_id, spend_id, owner_2) - - -def test_challenge_in_flight_exit_input_spent_unrelated_tx_should_fail(testlang): - owner_1, owner_2, amount = testlang.accounts[0], testlang.accounts[1], 100 - deposit_id_1 = testlang.deposit(owner_1, amount) - deposit_id_2 = testlang.deposit(owner_1, amount) - spend_id = testlang.spend_utxo([deposit_id_1], [owner_1], [(owner_1.address, NULL_ADDRESS, 100)]) - unrelated_spend_id = testlang.spend_utxo([deposit_id_2], [owner_1], [(owner_1.address, NULL_ADDRESS, 100)]) - testlang.start_in_flight_exit(spend_id) - testlang.piggyback_in_flight_exit_input(spend_id, 0, owner_1) - testlang.forward_to_period(2) - - with pytest.raises(TransactionFailed): - testlang.challenge_in_flight_exit_input_spent(spend_id, unrelated_spend_id, owner_2) - - -def test_challenge_in_flight_exit_input_spent_invalid_signature_should_fail(testlang): - owner_1, owner_2, amount = testlang.accounts[0], testlang.accounts[1], 100 - deposit_id = testlang.deposit(owner_1, amount) - spend_id = testlang.spend_utxo([deposit_id], [owner_1], [(owner_1.address, NULL_ADDRESS, 100)]) - double_spend_id = testlang.spend_utxo([deposit_id], [owner_2], [(owner_2.address, NULL_ADDRESS, 100)], force_invalid=True) - testlang.start_in_flight_exit(spend_id) - testlang.piggyback_in_flight_exit_input(spend_id, 0, owner_1) - testlang.forward_to_period(2) - - with pytest.raises(TransactionFailed): - testlang.challenge_in_flight_exit_input_spent(spend_id, double_spend_id, owner_2) From 5ee9e3936bb329347408b9db77273527a77c3bc3 Mon Sep 17 00:00:00 2001 From: boolafish Date: Wed, 4 Mar 2020 17:54:45 +0900 Subject: [PATCH 2/8] test: integ test for all challenge indexes on IFE input spend Test in previous commit failed to cover the situation that would fail the contract with wrong index. This one test the index from another side (challenge input index) and it works to break the contract. Without the fix, it would result in "VM Exception while processing transaction: revert Output index out of bounds" error from the EVM. --- ...st_challenge_in_flight_exit_input_spent.py | 105 +++++++++++++----- 1 file changed, 75 insertions(+), 30 deletions(-) diff --git a/plasma_framework/python_tests/tests/contracts/root_chain/test_challenge_in_flight_exit_input_spent.py b/plasma_framework/python_tests/tests/contracts/root_chain/test_challenge_in_flight_exit_input_spent.py index ac3e067bd..79d9695a0 100644 --- a/plasma_framework/python_tests/tests/contracts/root_chain/test_challenge_in_flight_exit_input_spent.py +++ b/plasma_framework/python_tests/tests/contracts/root_chain/test_challenge_in_flight_exit_input_spent.py @@ -22,49 +22,94 @@ -@pytest.mark.parametrize("num_outputs", [1, 2, 3, 4]) -def test_challenge_in_flight_exit_should_succeed_for_all_output_indexes(testlang, num_outputs): - deposit_amount = 100 - deposit_id = testlang.deposit(testlang.accounts[0], deposit_amount) - - owners = [] - outputs = [] - tx_output_amount = deposit_amount // num_outputs - for i in range(0, num_outputs): - owners.append(testlang.accounts[i]) - outputs.append((testlang.accounts[i].address, NULL_ADDRESS, tx_output_amount)) - - input_tx_id = testlang.spend_utxo([deposit_id], owners, outputs=outputs) - - blknum, tx_index, _ = decode_utxo_id(input_tx_id) - double_spend_output_index = num_outputs - 1 - double_spend_owner = owners[double_spend_output_index] - double_spend_utxo = encode_utxo_id(blknum, tx_index, double_spend_output_index) - - ife_output_amount = tx_output_amount // 2 +# @pytest.mark.parametrize("num_outputs", [1, 2, 3, 4]) +# def test_challenge_in_flight_exit_should_succeed_for_all_output_indexes(testlang, num_outputs): +# deposit_amount = 100 +# deposit_id = testlang.deposit(testlang.accounts[0], deposit_amount) + +# owners = [] +# outputs = [] +# tx_output_amount = deposit_amount // num_outputs +# for i in range(0, num_outputs): +# owners.append(testlang.accounts[i]) +# outputs.append((testlang.accounts[i].address, NULL_ADDRESS, tx_output_amount)) + +# input_tx_id = testlang.spend_utxo([deposit_id], owners, outputs=outputs) + +# blknum, tx_index, _ = decode_utxo_id(input_tx_id) +# double_spend_output_index = num_outputs - 1 +# double_spend_owner = owners[double_spend_output_index] +# double_spend_utxo = encode_utxo_id(blknum, tx_index, double_spend_output_index) + +# ife_output_amount = tx_output_amount // 2 +# ife_tx_id = testlang.spend_utxo( +# [double_spend_utxo], +# [double_spend_owner], +# [(owners[0].address, NULL_ADDRESS, ife_output_amount)] +# ) + +# challenge_tx_id = testlang.spend_utxo( +# [double_spend_utxo], +# [double_spend_owner], +# [ +# (owners[0].address, NULL_ADDRESS, ife_output_amount), +# (owners[0].address, NULL_ADDRESS, ife_output_amount) +# ], +# force_invalid=True +# ) + +# testlang.start_in_flight_exit(ife_tx_id) +# testlang.piggyback_in_flight_exit_input(ife_tx_id, 0, double_spend_owner) +# testlang.forward_to_period(1) +# testlang.challenge_in_flight_exit_input_spent(ife_tx_id, challenge_tx_id, double_spend_owner) + +# in_flight_exit = testlang.get_in_flight_exit(ife_tx_id) +# assert not in_flight_exit.input_piggybacked(double_spend_output_index) + + +@pytest.mark.parametrize("challenge_index", [0, 1, 2, 3]) +def test_challenge_in_flight_exit_should_succeed_for_all_challenge_indexes(testlang, challenge_index): + MAX_INDEX_SIZE = 4 + alice, bob, carol = testlang.accounts[0], testlang.accounts[1], testlang.accounts[2] + amount = 100 + + deposit_id = testlang.deposit(alice, amount) + + input_tx_id = testlang.spend_utxo( + [deposit_id], + [testlang.accounts[0]], + outputs=[(alice.address, NULL_ADDRESS, amount)] + ) + ife_tx_id = testlang.spend_utxo( - [double_spend_utxo], - [double_spend_owner], - [(owners[0].address, NULL_ADDRESS, ife_output_amount)] + [input_tx_id], + [alice], + [(bob.address, NULL_ADDRESS, amount)] ) + inputs = [] + for i in range(0, MAX_INDEX_SIZE): + if i == challenge_index: + inputs.append(input_tx_id) + else: + inputs.append(testlang.deposit(alice, amount)) + challenge_tx_id = testlang.spend_utxo( - [double_spend_utxo], - [double_spend_owner], + inputs, + [alice for i in range(0, MAX_INDEX_SIZE)], [ - (owners[0].address, NULL_ADDRESS, ife_output_amount), - (owners[0].address, NULL_ADDRESS, ife_output_amount) + (carol.address, NULL_ADDRESS, amount), ], force_invalid=True ) testlang.start_in_flight_exit(ife_tx_id) - testlang.piggyback_in_flight_exit_input(ife_tx_id, 0, double_spend_owner) + testlang.piggyback_in_flight_exit_input(ife_tx_id, 0, alice) testlang.forward_to_period(1) - testlang.challenge_in_flight_exit_input_spent(ife_tx_id, challenge_tx_id, double_spend_owner) + testlang.challenge_in_flight_exit_input_spent(ife_tx_id, challenge_tx_id, carol) in_flight_exit = testlang.get_in_flight_exit(ife_tx_id) - assert not in_flight_exit.input_piggybacked(double_spend_output_index) + assert not in_flight_exit.input_piggybacked(0) # def test_challenge_in_flight_exit_input_spent_not_piggybacked_should_fail(testlang): From 5a654fa168b43e33c810dacf1d950e116b623124 Mon Sep 17 00:00:00 2001 From: boolafish Date: Wed, 4 Mar 2020 17:57:47 +0900 Subject: [PATCH 3/8] refactor: uncomment tests --- ...st_challenge_in_flight_exit_input_spent.py | 200 +++++++++--------- 1 file changed, 100 insertions(+), 100 deletions(-) diff --git a/plasma_framework/python_tests/tests/contracts/root_chain/test_challenge_in_flight_exit_input_spent.py b/plasma_framework/python_tests/tests/contracts/root_chain/test_challenge_in_flight_exit_input_spent.py index 79d9695a0..780ad655e 100644 --- a/plasma_framework/python_tests/tests/contracts/root_chain/test_challenge_in_flight_exit_input_spent.py +++ b/plasma_framework/python_tests/tests/contracts/root_chain/test_challenge_in_flight_exit_input_spent.py @@ -4,67 +4,67 @@ from plasma_core.utils.transactions import decode_utxo_id, encode_utxo_id -# # should succeed even when phase 2 of in-flight exit is over -# @pytest.mark.parametrize("period", [1, 2, 3, 4]) -# def test_challenge_in_flight_exit_input_spent_should_succeed_in_all_periods(testlang, period): -# owner_1, owner_2, amount = testlang.accounts[0], testlang.accounts[1], 100 -# deposit_id = testlang.deposit(owner_1, amount) -# spend_id = testlang.spend_utxo([deposit_id], [owner_1], [(owner_1.address, NULL_ADDRESS, 100)]) -# double_spend_id = testlang.spend_utxo([deposit_id], [owner_1], [(owner_2.address, NULL_ADDRESS, 100)], force_invalid=True) -# testlang.start_in_flight_exit(spend_id) -# testlang.piggyback_in_flight_exit_input(spend_id, 0, owner_1) -# testlang.forward_to_period(period) - -# testlang.challenge_in_flight_exit_input_spent(spend_id, double_spend_id, owner_2) - -# in_flight_exit = testlang.get_in_flight_exit(spend_id) -# assert not in_flight_exit.input_piggybacked(0) - - - -# @pytest.mark.parametrize("num_outputs", [1, 2, 3, 4]) -# def test_challenge_in_flight_exit_should_succeed_for_all_output_indexes(testlang, num_outputs): -# deposit_amount = 100 -# deposit_id = testlang.deposit(testlang.accounts[0], deposit_amount) - -# owners = [] -# outputs = [] -# tx_output_amount = deposit_amount // num_outputs -# for i in range(0, num_outputs): -# owners.append(testlang.accounts[i]) -# outputs.append((testlang.accounts[i].address, NULL_ADDRESS, tx_output_amount)) - -# input_tx_id = testlang.spend_utxo([deposit_id], owners, outputs=outputs) - -# blknum, tx_index, _ = decode_utxo_id(input_tx_id) -# double_spend_output_index = num_outputs - 1 -# double_spend_owner = owners[double_spend_output_index] -# double_spend_utxo = encode_utxo_id(blknum, tx_index, double_spend_output_index) - -# ife_output_amount = tx_output_amount // 2 -# ife_tx_id = testlang.spend_utxo( -# [double_spend_utxo], -# [double_spend_owner], -# [(owners[0].address, NULL_ADDRESS, ife_output_amount)] -# ) - -# challenge_tx_id = testlang.spend_utxo( -# [double_spend_utxo], -# [double_spend_owner], -# [ -# (owners[0].address, NULL_ADDRESS, ife_output_amount), -# (owners[0].address, NULL_ADDRESS, ife_output_amount) -# ], -# force_invalid=True -# ) - -# testlang.start_in_flight_exit(ife_tx_id) -# testlang.piggyback_in_flight_exit_input(ife_tx_id, 0, double_spend_owner) -# testlang.forward_to_period(1) -# testlang.challenge_in_flight_exit_input_spent(ife_tx_id, challenge_tx_id, double_spend_owner) - -# in_flight_exit = testlang.get_in_flight_exit(ife_tx_id) -# assert not in_flight_exit.input_piggybacked(double_spend_output_index) +# should succeed even when phase 2 of in-flight exit is over +@pytest.mark.parametrize("period", [1, 2, 3, 4]) +def test_challenge_in_flight_exit_input_spent_should_succeed_in_all_periods(testlang, period): + owner_1, owner_2, amount = testlang.accounts[0], testlang.accounts[1], 100 + deposit_id = testlang.deposit(owner_1, amount) + spend_id = testlang.spend_utxo([deposit_id], [owner_1], [(owner_1.address, NULL_ADDRESS, 100)]) + double_spend_id = testlang.spend_utxo([deposit_id], [owner_1], [(owner_2.address, NULL_ADDRESS, 100)], force_invalid=True) + testlang.start_in_flight_exit(spend_id) + testlang.piggyback_in_flight_exit_input(spend_id, 0, owner_1) + testlang.forward_to_period(period) + + testlang.challenge_in_flight_exit_input_spent(spend_id, double_spend_id, owner_2) + + in_flight_exit = testlang.get_in_flight_exit(spend_id) + assert not in_flight_exit.input_piggybacked(0) + + + +@pytest.mark.parametrize("num_outputs", [1, 2, 3, 4]) +def test_challenge_in_flight_exit_should_succeed_for_all_output_indexes(testlang, num_outputs): + deposit_amount = 100 + deposit_id = testlang.deposit(testlang.accounts[0], deposit_amount) + + owners = [] + outputs = [] + tx_output_amount = deposit_amount // num_outputs + for i in range(0, num_outputs): + owners.append(testlang.accounts[i]) + outputs.append((testlang.accounts[i].address, NULL_ADDRESS, tx_output_amount)) + + input_tx_id = testlang.spend_utxo([deposit_id], owners, outputs=outputs) + + blknum, tx_index, _ = decode_utxo_id(input_tx_id) + double_spend_output_index = num_outputs - 1 + double_spend_owner = owners[double_spend_output_index] + double_spend_utxo = encode_utxo_id(blknum, tx_index, double_spend_output_index) + + ife_output_amount = tx_output_amount // 2 + ife_tx_id = testlang.spend_utxo( + [double_spend_utxo], + [double_spend_owner], + [(owners[0].address, NULL_ADDRESS, ife_output_amount)] + ) + + challenge_tx_id = testlang.spend_utxo( + [double_spend_utxo], + [double_spend_owner], + [ + (owners[0].address, NULL_ADDRESS, ife_output_amount), + (owners[0].address, NULL_ADDRESS, ife_output_amount) + ], + force_invalid=True + ) + + testlang.start_in_flight_exit(ife_tx_id) + testlang.piggyback_in_flight_exit_input(ife_tx_id, 0, double_spend_owner) + testlang.forward_to_period(1) + testlang.challenge_in_flight_exit_input_spent(ife_tx_id, challenge_tx_id, double_spend_owner) + + in_flight_exit = testlang.get_in_flight_exit(ife_tx_id) + assert not in_flight_exit.input_piggybacked(double_spend_output_index) @pytest.mark.parametrize("challenge_index", [0, 1, 2, 3]) @@ -112,53 +112,53 @@ def test_challenge_in_flight_exit_should_succeed_for_all_challenge_indexes(testl assert not in_flight_exit.input_piggybacked(0) -# def test_challenge_in_flight_exit_input_spent_not_piggybacked_should_fail(testlang): -# owner_1, owner_2, amount = testlang.accounts[0], testlang.accounts[1], 100 -# deposit_id = testlang.deposit(owner_1, amount) -# spend_id = testlang.spend_utxo([deposit_id], [owner_1], [(owner_1.address, NULL_ADDRESS, 100)]) -# double_spend_id = testlang.spend_utxo([deposit_id], [owner_1], [(owner_2.address, NULL_ADDRESS, 100)], force_invalid=True) -# testlang.start_in_flight_exit(spend_id) -# testlang.forward_to_period(2) +def test_challenge_in_flight_exit_input_spent_not_piggybacked_should_fail(testlang): + owner_1, owner_2, amount = testlang.accounts[0], testlang.accounts[1], 100 + deposit_id = testlang.deposit(owner_1, amount) + spend_id = testlang.spend_utxo([deposit_id], [owner_1], [(owner_1.address, NULL_ADDRESS, 100)]) + double_spend_id = testlang.spend_utxo([deposit_id], [owner_1], [(owner_2.address, NULL_ADDRESS, 100)], force_invalid=True) + testlang.start_in_flight_exit(spend_id) + testlang.forward_to_period(2) -# with pytest.raises(TransactionFailed): -# testlang.challenge_in_flight_exit_input_spent(spend_id, double_spend_id, owner_2) + with pytest.raises(TransactionFailed): + testlang.challenge_in_flight_exit_input_spent(spend_id, double_spend_id, owner_2) -# def test_challenge_in_flight_exit_input_spent_same_tx_should_fail(testlang): -# owner_1, owner_2, amount = testlang.accounts[0], testlang.accounts[1], 100 -# deposit_id = testlang.deposit(owner_1, amount) -# spend_id = testlang.spend_utxo([deposit_id], [owner_1], [(owner_1.address, NULL_ADDRESS, 100)]) -# testlang.start_in_flight_exit(spend_id) -# testlang.piggyback_in_flight_exit_input(spend_id, 0, owner_1) -# testlang.forward_to_period(2) +def test_challenge_in_flight_exit_input_spent_same_tx_should_fail(testlang): + owner_1, owner_2, amount = testlang.accounts[0], testlang.accounts[1], 100 + deposit_id = testlang.deposit(owner_1, amount) + spend_id = testlang.spend_utxo([deposit_id], [owner_1], [(owner_1.address, NULL_ADDRESS, 100)]) + testlang.start_in_flight_exit(spend_id) + testlang.piggyback_in_flight_exit_input(spend_id, 0, owner_1) + testlang.forward_to_period(2) -# with pytest.raises(TransactionFailed): -# testlang.challenge_in_flight_exit_input_spent(spend_id, spend_id, owner_2) + with pytest.raises(TransactionFailed): + testlang.challenge_in_flight_exit_input_spent(spend_id, spend_id, owner_2) -# def test_challenge_in_flight_exit_input_spent_unrelated_tx_should_fail(testlang): -# owner_1, owner_2, amount = testlang.accounts[0], testlang.accounts[1], 100 -# deposit_id_1 = testlang.deposit(owner_1, amount) -# deposit_id_2 = testlang.deposit(owner_1, amount) -# spend_id = testlang.spend_utxo([deposit_id_1], [owner_1], [(owner_1.address, NULL_ADDRESS, 100)]) -# unrelated_spend_id = testlang.spend_utxo([deposit_id_2], [owner_1], [(owner_1.address, NULL_ADDRESS, 100)]) -# testlang.start_in_flight_exit(spend_id) -# testlang.piggyback_in_flight_exit_input(spend_id, 0, owner_1) -# testlang.forward_to_period(2) +def test_challenge_in_flight_exit_input_spent_unrelated_tx_should_fail(testlang): + owner_1, owner_2, amount = testlang.accounts[0], testlang.accounts[1], 100 + deposit_id_1 = testlang.deposit(owner_1, amount) + deposit_id_2 = testlang.deposit(owner_1, amount) + spend_id = testlang.spend_utxo([deposit_id_1], [owner_1], [(owner_1.address, NULL_ADDRESS, 100)]) + unrelated_spend_id = testlang.spend_utxo([deposit_id_2], [owner_1], [(owner_1.address, NULL_ADDRESS, 100)]) + testlang.start_in_flight_exit(spend_id) + testlang.piggyback_in_flight_exit_input(spend_id, 0, owner_1) + testlang.forward_to_period(2) -# with pytest.raises(TransactionFailed): -# testlang.challenge_in_flight_exit_input_spent(spend_id, unrelated_spend_id, owner_2) + with pytest.raises(TransactionFailed): + testlang.challenge_in_flight_exit_input_spent(spend_id, unrelated_spend_id, owner_2) -# def test_challenge_in_flight_exit_input_spent_invalid_signature_should_fail(testlang): -# owner_1, owner_2, amount = testlang.accounts[0], testlang.accounts[1], 100 -# deposit_id = testlang.deposit(owner_1, amount) -# spend_id = testlang.spend_utxo([deposit_id], [owner_1], [(owner_1.address, NULL_ADDRESS, 100)]) -# double_spend_id = testlang.spend_utxo([deposit_id], [owner_2], [(owner_2.address, NULL_ADDRESS, 100)], force_invalid=True) -# testlang.start_in_flight_exit(spend_id) -# testlang.piggyback_in_flight_exit_input(spend_id, 0, owner_1) -# testlang.forward_to_period(2) +def test_challenge_in_flight_exit_input_spent_invalid_signature_should_fail(testlang): + owner_1, owner_2, amount = testlang.accounts[0], testlang.accounts[1], 100 + deposit_id = testlang.deposit(owner_1, amount) + spend_id = testlang.spend_utxo([deposit_id], [owner_1], [(owner_1.address, NULL_ADDRESS, 100)]) + double_spend_id = testlang.spend_utxo([deposit_id], [owner_2], [(owner_2.address, NULL_ADDRESS, 100)], force_invalid=True) + testlang.start_in_flight_exit(spend_id) + testlang.piggyback_in_flight_exit_input(spend_id, 0, owner_1) + testlang.forward_to_period(2) -# with pytest.raises(TransactionFailed): -# testlang.challenge_in_flight_exit_input_spent(spend_id, double_spend_id, owner_2) + with pytest.raises(TransactionFailed): + testlang.challenge_in_flight_exit_input_spent(spend_id, double_spend_id, owner_2) From 62b7d32f2079aab45a21cb92f9f19834d8d777d9 Mon Sep 17 00:00:00 2001 From: boolafish Date: Wed, 4 Mar 2020 17:58:02 +0900 Subject: [PATCH 4/8] fix: ife spend with wrong index This commit is the fix on the contract for input spend challenge --- .../payment/controllers/PaymentChallengeIFEInputSpent.sol | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plasma_framework/contracts/src/exits/payment/controllers/PaymentChallengeIFEInputSpent.sol b/plasma_framework/contracts/src/exits/payment/controllers/PaymentChallengeIFEInputSpent.sol index 9e9df96a7..83e1901e4 100644 --- a/plasma_framework/contracts/src/exits/payment/controllers/PaymentChallengeIFEInputSpent.sol +++ b/plasma_framework/contracts/src/exits/payment/controllers/PaymentChallengeIFEInputSpent.sol @@ -136,7 +136,8 @@ library PaymentChallengeIFEInputSpent { GenericTransaction.Transaction memory challengingTx = GenericTransaction.decode(data.args.challengingTx); GenericTransaction.Transaction memory inputTx = GenericTransaction.decode(data.args.inputTx); - GenericTransaction.Output memory output = GenericTransaction.getOutput(inputTx, data.args.challengingTxInputIndex); + PosLib.Position memory utxoPos = PosLib.decode(data.args.inputUtxoPos); + GenericTransaction.Output memory output = GenericTransaction.getOutput(inputTx, utxoPos.outputIndex); ISpendingCondition condition = data.controller.spendingConditionRegistry.spendingConditions( output.outputType, challengingTx.txType From 4e1b643ad844544a8bf19b5985e66002a01f62e9 Mon Sep 17 00:00:00 2001 From: boolafish Date: Wed, 4 Mar 2020 18:19:43 +0900 Subject: [PATCH 5/8] style: fix linter --- .../root_chain/test_challenge_in_flight_exit_input_spent.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/plasma_framework/python_tests/tests/contracts/root_chain/test_challenge_in_flight_exit_input_spent.py b/plasma_framework/python_tests/tests/contracts/root_chain/test_challenge_in_flight_exit_input_spent.py index 780ad655e..cc57b9e9c 100644 --- a/plasma_framework/python_tests/tests/contracts/root_chain/test_challenge_in_flight_exit_input_spent.py +++ b/plasma_framework/python_tests/tests/contracts/root_chain/test_challenge_in_flight_exit_input_spent.py @@ -21,7 +21,6 @@ def test_challenge_in_flight_exit_input_spent_should_succeed_in_all_periods(test assert not in_flight_exit.input_piggybacked(0) - @pytest.mark.parametrize("num_outputs", [1, 2, 3, 4]) def test_challenge_in_flight_exit_should_succeed_for_all_output_indexes(testlang, num_outputs): deposit_amount = 100 @@ -161,4 +160,3 @@ def test_challenge_in_flight_exit_input_spent_invalid_signature_should_fail(test with pytest.raises(TransactionFailed): testlang.challenge_in_flight_exit_input_spent(spend_id, double_spend_id, owner_2) - From 4167bcb307ddb8ae941a92ada40381d800e242bc Mon Sep 17 00:00:00 2001 From: boolafish Date: Wed, 4 Mar 2020 20:58:23 +0900 Subject: [PATCH 6/8] fix: test description on period --- .../root_chain/test_challenge_in_flight_exit_input_spent.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plasma_framework/python_tests/tests/contracts/root_chain/test_challenge_in_flight_exit_input_spent.py b/plasma_framework/python_tests/tests/contracts/root_chain/test_challenge_in_flight_exit_input_spent.py index cc57b9e9c..227caaa82 100644 --- a/plasma_framework/python_tests/tests/contracts/root_chain/test_challenge_in_flight_exit_input_spent.py +++ b/plasma_framework/python_tests/tests/contracts/root_chain/test_challenge_in_flight_exit_input_spent.py @@ -5,8 +5,8 @@ # should succeed even when phase 2 of in-flight exit is over -@pytest.mark.parametrize("period", [1, 2, 3, 4]) -def test_challenge_in_flight_exit_input_spent_should_succeed_in_all_periods(testlang, period): +@pytest.mark.parametrize("period", [1, 2, 3]) +def test_challenge_in_flight_exit_input_spent_should_succeed_after_first_period_passes(testlang, period): owner_1, owner_2, amount = testlang.accounts[0], testlang.accounts[1], 100 deposit_id = testlang.deposit(owner_1, amount) spend_id = testlang.spend_utxo([deposit_id], [owner_1], [(owner_1.address, NULL_ADDRESS, 100)]) From 2517a88e482e32a640715981b79f7da487304c36 Mon Sep 17 00:00:00 2001 From: boolafish Date: Thu, 5 Mar 2020 15:26:32 +0900 Subject: [PATCH 7/8] refactor: merge previous two tests into one with test parameters 1. kudo Piotr's idea to parametize the inputs for indexes at once 1. rename the period test a bit as just realize that it can work with period 0 too Piotr's PR: https://github.com/omisego/plasma-contracts/pull/594 --- ...st_challenge_in_flight_exit_input_spent.py | 79 ++++++------------- .../tests/tests_utils/constants.py | 3 + 2 files changed, 25 insertions(+), 57 deletions(-) diff --git a/plasma_framework/python_tests/tests/contracts/root_chain/test_challenge_in_flight_exit_input_spent.py b/plasma_framework/python_tests/tests/contracts/root_chain/test_challenge_in_flight_exit_input_spent.py index 227caaa82..a1ee987f5 100644 --- a/plasma_framework/python_tests/tests/contracts/root_chain/test_challenge_in_flight_exit_input_spent.py +++ b/plasma_framework/python_tests/tests/contracts/root_chain/test_challenge_in_flight_exit_input_spent.py @@ -2,11 +2,12 @@ from eth_tester.exceptions import TransactionFailed from plasma_core.constants import NULL_ADDRESS from plasma_core.utils.transactions import decode_utxo_id, encode_utxo_id +from tests_utils.constants import PAYMENT_TX_MAX_INPUT_SIZE, PAYMENT_TX_MAX_OUTPUT_SIZE # should succeed even when phase 2 of in-flight exit is over -@pytest.mark.parametrize("period", [1, 2, 3]) -def test_challenge_in_flight_exit_input_spent_should_succeed_after_first_period_passes(testlang, period): +@pytest.mark.parametrize("period", [0, 1, 2, 3]) +def test_challenge_in_flight_exit_input_spent_should_succeed_for_all_periods(testlang, period): owner_1, owner_2, amount = testlang.accounts[0], testlang.accounts[1], 100 deposit_id = testlang.deposit(owner_1, amount) spend_id = testlang.spend_utxo([deposit_id], [owner_1], [(owner_1.address, NULL_ADDRESS, 100)]) @@ -21,22 +22,25 @@ def test_challenge_in_flight_exit_input_spent_should_succeed_after_first_period_ assert not in_flight_exit.input_piggybacked(0) -@pytest.mark.parametrize("num_outputs", [1, 2, 3, 4]) -def test_challenge_in_flight_exit_should_succeed_for_all_output_indexes(testlang, num_outputs): +@pytest.mark.parametrize( + "double_spend_output_index,challenge_input_index", + [(i, j) for i in range(0, PAYMENT_TX_MAX_OUTPUT_SIZE) for j in range(0, PAYMENT_TX_MAX_INPUT_SIZE)] +) +def test_challenge_in_flight_exit_input_spent_should_succeed_for_all_indices( + testlang, double_spend_output_index, challenge_input_index +): deposit_amount = 100 deposit_id = testlang.deposit(testlang.accounts[0], deposit_amount) owners = [] outputs = [] - tx_output_amount = deposit_amount // num_outputs - for i in range(0, num_outputs): + tx_output_amount = deposit_amount // PAYMENT_TX_MAX_OUTPUT_SIZE + for i in range(0, PAYMENT_TX_MAX_OUTPUT_SIZE): owners.append(testlang.accounts[i]) outputs.append((testlang.accounts[i].address, NULL_ADDRESS, tx_output_amount)) input_tx_id = testlang.spend_utxo([deposit_id], owners, outputs=outputs) - blknum, tx_index, _ = decode_utxo_id(input_tx_id) - double_spend_output_index = num_outputs - 1 double_spend_owner = owners[double_spend_output_index] double_spend_utxo = encode_utxo_id(blknum, tx_index, double_spend_output_index) @@ -47,68 +51,29 @@ def test_challenge_in_flight_exit_should_succeed_for_all_output_indexes(testlang [(owners[0].address, NULL_ADDRESS, ife_output_amount)] ) - challenge_tx_id = testlang.spend_utxo( - [double_spend_utxo], - [double_spend_owner], - [ - (owners[0].address, NULL_ADDRESS, ife_output_amount), - (owners[0].address, NULL_ADDRESS, ife_output_amount) - ], - force_invalid=True - ) - - testlang.start_in_flight_exit(ife_tx_id) - testlang.piggyback_in_flight_exit_input(ife_tx_id, 0, double_spend_owner) - testlang.forward_to_period(1) - testlang.challenge_in_flight_exit_input_spent(ife_tx_id, challenge_tx_id, double_spend_owner) - - in_flight_exit = testlang.get_in_flight_exit(ife_tx_id) - assert not in_flight_exit.input_piggybacked(double_spend_output_index) - - -@pytest.mark.parametrize("challenge_index", [0, 1, 2, 3]) -def test_challenge_in_flight_exit_should_succeed_for_all_challenge_indexes(testlang, challenge_index): - MAX_INDEX_SIZE = 4 - alice, bob, carol = testlang.accounts[0], testlang.accounts[1], testlang.accounts[2] - amount = 100 - - deposit_id = testlang.deposit(alice, amount) - - input_tx_id = testlang.spend_utxo( - [deposit_id], - [testlang.accounts[0]], - outputs=[(alice.address, NULL_ADDRESS, amount)] - ) - - ife_tx_id = testlang.spend_utxo( - [input_tx_id], - [alice], - [(bob.address, NULL_ADDRESS, amount)] - ) - inputs = [] - for i in range(0, MAX_INDEX_SIZE): - if i == challenge_index: - inputs.append(input_tx_id) + for i in range(0, PAYMENT_TX_MAX_INPUT_SIZE): + if i == challenge_input_index: + inputs.append(double_spend_utxo) else: - inputs.append(testlang.deposit(alice, amount)) + inputs.append(testlang.deposit(double_spend_owner, tx_output_amount)) challenge_tx_id = testlang.spend_utxo( inputs, - [alice for i in range(0, MAX_INDEX_SIZE)], + [double_spend_owner for i in range(0, PAYMENT_TX_MAX_INPUT_SIZE)], [ - (carol.address, NULL_ADDRESS, amount), + (double_spend_owner.address, NULL_ADDRESS, tx_output_amount), ], force_invalid=True ) testlang.start_in_flight_exit(ife_tx_id) - testlang.piggyback_in_flight_exit_input(ife_tx_id, 0, alice) - testlang.forward_to_period(1) - testlang.challenge_in_flight_exit_input_spent(ife_tx_id, challenge_tx_id, carol) + testlang.piggyback_in_flight_exit_input(ife_tx_id, 0, double_spend_owner) + testlang.challenge_in_flight_exit_input_spent(ife_tx_id, challenge_tx_id, double_spend_owner) in_flight_exit = testlang.get_in_flight_exit(ife_tx_id) - assert not in_flight_exit.input_piggybacked(0) + for i in range(0, PAYMENT_TX_MAX_INPUT_SIZE): + assert not in_flight_exit.input_piggybacked(i) def test_challenge_in_flight_exit_input_spent_not_piggybacked_should_fail(testlang): diff --git a/plasma_framework/python_tests/tests/tests_utils/constants.py b/plasma_framework/python_tests/tests/tests_utils/constants.py index 87b40f243..bb06e050f 100644 --- a/plasma_framework/python_tests/tests/tests_utils/constants.py +++ b/plasma_framework/python_tests/tests/tests_utils/constants.py @@ -11,3 +11,6 @@ START_GAS = GAS_LIMIT - 1000000 INITIAL_ETH = 10000 * 10 ** 18 + +PAYMENT_TX_MAX_INPUT_SIZE = 4 +PAYMENT_TX_MAX_OUTPUT_SIZE = 4 From e9edf672e1c667ebe4feabf07f7b9baeaf2c846f Mon Sep 17 00:00:00 2001 From: boolafish Date: Thu, 5 Mar 2020 15:55:54 +0900 Subject: [PATCH 8/8] refactor: small changes on genreate list --- ...st_challenge_in_flight_exit_input_spent.py | 32 ++++++++----------- 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/plasma_framework/python_tests/tests/contracts/root_chain/test_challenge_in_flight_exit_input_spent.py b/plasma_framework/python_tests/tests/contracts/root_chain/test_challenge_in_flight_exit_input_spent.py index a1ee987f5..6f5feb7bf 100644 --- a/plasma_framework/python_tests/tests/contracts/root_chain/test_challenge_in_flight_exit_input_spent.py +++ b/plasma_framework/python_tests/tests/contracts/root_chain/test_challenge_in_flight_exit_input_spent.py @@ -24,31 +24,27 @@ def test_challenge_in_flight_exit_input_spent_should_succeed_for_all_periods(tes @pytest.mark.parametrize( "double_spend_output_index,challenge_input_index", - [(i, j) for i in range(0, PAYMENT_TX_MAX_OUTPUT_SIZE) for j in range(0, PAYMENT_TX_MAX_INPUT_SIZE)] + [(i, j) for i in range(PAYMENT_TX_MAX_OUTPUT_SIZE) for j in range(PAYMENT_TX_MAX_INPUT_SIZE)] ) def test_challenge_in_flight_exit_input_spent_should_succeed_for_all_indices( testlang, double_spend_output_index, challenge_input_index ): + alice, bob, carol = testlang.accounts[0], testlang.accounts[1], testlang.accounts[2] deposit_amount = 100 - deposit_id = testlang.deposit(testlang.accounts[0], deposit_amount) + deposit_id = testlang.deposit(alice, deposit_amount) - owners = [] - outputs = [] tx_output_amount = deposit_amount // PAYMENT_TX_MAX_OUTPUT_SIZE - for i in range(0, PAYMENT_TX_MAX_OUTPUT_SIZE): - owners.append(testlang.accounts[i]) - outputs.append((testlang.accounts[i].address, NULL_ADDRESS, tx_output_amount)) + outputs = [(alice.address, NULL_ADDRESS, tx_output_amount)] * PAYMENT_TX_MAX_OUTPUT_SIZE - input_tx_id = testlang.spend_utxo([deposit_id], owners, outputs=outputs) + input_tx_id = testlang.spend_utxo([deposit_id], [alice], outputs=outputs) blknum, tx_index, _ = decode_utxo_id(input_tx_id) - double_spend_owner = owners[double_spend_output_index] double_spend_utxo = encode_utxo_id(blknum, tx_index, double_spend_output_index) - ife_output_amount = tx_output_amount // 2 + ife_output_amount = tx_output_amount ife_tx_id = testlang.spend_utxo( [double_spend_utxo], - [double_spend_owner], - [(owners[0].address, NULL_ADDRESS, ife_output_amount)] + [alice], + [(bob.address, NULL_ADDRESS, ife_output_amount)] ) inputs = [] @@ -56,20 +52,20 @@ def test_challenge_in_flight_exit_input_spent_should_succeed_for_all_indices( if i == challenge_input_index: inputs.append(double_spend_utxo) else: - inputs.append(testlang.deposit(double_spend_owner, tx_output_amount)) + inputs.append(testlang.deposit(alice, tx_output_amount)) challenge_tx_id = testlang.spend_utxo( inputs, - [double_spend_owner for i in range(0, PAYMENT_TX_MAX_INPUT_SIZE)], + [alice] * PAYMENT_TX_MAX_INPUT_SIZE, [ - (double_spend_owner.address, NULL_ADDRESS, tx_output_amount), + (carol.address, NULL_ADDRESS, tx_output_amount), ], force_invalid=True ) - testlang.start_in_flight_exit(ife_tx_id) - testlang.piggyback_in_flight_exit_input(ife_tx_id, 0, double_spend_owner) - testlang.challenge_in_flight_exit_input_spent(ife_tx_id, challenge_tx_id, double_spend_owner) + testlang.start_in_flight_exit(ife_tx_id, sender=bob) + testlang.piggyback_in_flight_exit_input(ife_tx_id, 0, alice) + testlang.challenge_in_flight_exit_input_spent(ife_tx_id, challenge_tx_id, carol) in_flight_exit = testlang.get_in_flight_exit(ife_tx_id) for i in range(0, PAYMENT_TX_MAX_INPUT_SIZE):