Skip to content
This repository has been archived by the owner on Aug 23, 2020. It is now read-only.

Test: (In)valid bundle regressions #1784

Open
wants to merge 29 commits into
base: dev
Choose a base branch
from

Conversation

kwek20
Copy link
Contributor

@kwek20 kwek20 commented Mar 5, 2020

Description

Adds different cases for bundles which can be confirmed by the tangle

Fixes #1590

1590 describes 2 test which we currently cannot do in a regression test, but have as a unit test.

  • And at least one incomplete but valid bundle (missing an index) approves the former
    Already exists: BundleValidatorTest.validateSemanticsOfBundleWithMissingTransaction
  • And at least one invalid bundle approves the former. (send without input)
    Already exists: BundleValidatorTest.validationModeAllWithInvalidBundle

These regression tests are prepared in the file 2_disabled_transaction_tests.feature and will be usable once we have white flag implemented.

We need 2 changes to the regression tests:

  • Updated snapshots.txt for machine 2 (@DyrellC has the database)
  • Change the installation of the iota python library to the following: pip install pyota[pow] (This is required for custom pow for the test with the split bundle)

Snapshot changes are the following 2 additions:

SIDKDWQWWJHTWWBNTMQ9ERZJMCJAKWUDYBWDC9MRLX9JNBU9IPBVZCPBXQNSWWYHOBBYCDLTEIGLFOCCD;1000000
TXMPVKBDFXCCSUBRSAR9BZ9PNZRWL9NRHBIZJJEG9BHQGETCZKBVXWK9GACBJNGSKQIUZGQ9IDDDYJBZC;2000000

And corresponding removal to the rest address

Type of change

  • Enhancement/Test (a non-breaking change which adds functionality)

How Has This Been Tested?

Ran the tests locally.
Buildkite will fail as the database and installation isnt changed in buildkite yet

@kwek20 kwek20 requested review from GalRogozinski and DyrellC March 5, 2020 15:41
@kwek20 kwek20 changed the base branch from dev to release-v1.9.0 March 5, 2020 15:41
@@ -87,3 +87,106 @@ Feature: Test transaction confirmation
| keys | values | type |
| states | False | boolListMixed |

Scenario: Valid value transfer bundle that doesnt affect ledger state
We want to ascertain that ledger state is always calculated correctly.
Even in the presence of a bundle that hadnales funds but without chaning address
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Even in the presence of a bundle that hadnales funds but without chaning address
Even in the presence of a bundle that handles funds but without changing address

Comment on lines 121 to 126
Given "getBalances" is called on "nodeA-m2" with:
|keys |values |type |
|addresses |FAKE_SPEND_ADDRESSES |staticList |

Then the response for "getBalances" should return with:
|keys |values |type |
|balances |0 |int |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am trying to read this test intentionally without looking at the issue

I don't really understand where FAKE_SPEND_ADDRESSES is coming from
Don't understand exactly what you are testing

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FAKE_SPEND_ADDRESSES is coming from staticList, which is static_vals.py, like all other static values.

What i'm testing is described in the issue ticket, basically sending nonexistant iota backand forth, causing 0 ledger changes after applying. Which is a valid case and should be allowed. Therefor we should test it :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't clear
I meant that by reading Gherkin, I see this value only mentioned once

I don't think I can understand why the actions you are doing before have a chance to mutate the balance on this address

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we immediately apply balance changes instead of only per bundle? BOOM test saves us as there is no balance


Then the response for "getBalances" should return with:
|keys |values |type |
|balances |1000 0 |intList |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is this trailing 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its an intList, we test the balance of 2 addresses, hence receiving 2 values


Scenario: Split transaction over 2 bundles
We want to ascertain that ledger state is always calculated correctly.
Even when we hsare a transaction over 2 different bundles
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hsare?

@kwek20 kwek20 force-pushed the invalid-bundle-regression branch from 363149e to a71e3f0 Compare March 10, 2020 17:51
@kwek20 kwek20 changed the base branch from release-v1.9.0 to dev March 10, 2020 17:52
@kwek20 kwek20 force-pushed the invalid-bundle-regression branch from a71e3f0 to c5bc8f6 Compare March 10, 2020 17:53
@kwek20 kwek20 changed the base branch from dev to release-v1.9.0 March 10, 2020 17:54
@kwek20 kwek20 changed the base branch from release-v1.9.0 to dev March 10, 2020 17:54
@kwek20 kwek20 force-pushed the invalid-bundle-regression branch from c5bc8f6 to b919e05 Compare March 10, 2020 18:00
@kwek20 kwek20 changed the base branch from dev to release-v1.9.0 March 10, 2020 18:00
@kwek20 kwek20 force-pushed the invalid-bundle-regression branch from b919e05 to 4244ea1 Compare March 10, 2020 18:14
@kwek20 kwek20 changed the base branch from release-v1.9.0 to dev March 11, 2020 14:59
@kwek20 kwek20 force-pushed the invalid-bundle-regression branch from 482d496 to f060918 Compare March 11, 2020 17:41
|value |0 |int |
|tag |ZERO9VALUE |string |

Then a value transaction which does not move funds is generated referencing the previous transaction with:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Then a value transaction which does not move funds is generated referencing the previous transaction with:
Then a value bundle which moves funds back and forth from an address is generated referencing the previous transaction with:

Comment on lines 162 to 163
We want to ascertain that ledger state is always calculated correctly.
Even when there is a transaction used in 2 different bundles
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Define clearly what a "split" bundle is

Comment on lines 93 to 110
Then "1" transaction is issued on "nodeA-m3" with:
|keys |values |type |
|address |TEST_ADDRESS |staticValue |
|value |0 |int |
|tag |ZERO9VALUE |string |

Then a value bundle which moves funds back and forth from an address is generated referencing the previous transaction with:
|keys |values |type |
|seed |THE_BANK |staticList |
|value |100 |int |
|tag |FAKE9VALUE |string |

Then a transaction is issued referencing the previous transaction
|keys |values |type |
|seed |THE_BANK |staticList |
|address |TEST_ADDRESS |staticValue |
|value |11 |int |
|tag |VALUE9TRANSACTION |string |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The flow of this scenario could be easier to understand if you use When, Given, And and Then more appropriately.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@step(r'a split bundle is generated referencing the previous transaction with:')
def create_split_bundle(step):
"""
Create a bundle that reuses the last transaction form another bundle in its own
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Create a bundle that reuses the last transaction form another bundle in its own
Create a bundle that reuses the last transaction from another bundle in its own

@@ -75,7 +75,7 @@ Feature: Test Bootstrapping With LS
|keys |values |type |
|state |True |bool |


@getNodeInfo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

huh?

Comment on lines 93 to 110
Then "1" transaction is issued on "nodeA-m3" with:
|keys |values |type |
|address |TEST_ADDRESS |staticValue |
|value |0 |int |
|tag |ZERO9VALUE |string |

Then a value bundle which moves funds back and forth from an address is generated referencing the previous transaction with:
|keys |values |type |
|seed |THE_BANK |staticList |
|value |100 |int |
|tag |FAKE9VALUE |string |

Then a transaction is issued referencing the previous transaction
|keys |values |type |
|seed |THE_BANK |staticList |
|address |TEST_ADDRESS |staticValue |
|value |11 |int |
|tag |VALUE9TRANSACTION |string |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines 1 to 75
Feature: Test transaction confirmation

Scenario: Invalid bundle doesnt affect ledger state
We want to ascertain that ledger state is always calculated correctly.
Even in the presence of invalid bundles being approved.

Given "1" transaction is issued on "nodeA-m3" with:
|keys |values |type |
|address |TEST_ADDRESS |staticValue |
|value |0 |int |
|tag |ZERO9VALUE |string |

Then an invalid bundle is generated referencing the previous transaction

Then a transaction is issued referencing the previous transaction
|keys |values |type |
|seed |THE_BANK |staticList |
|address |TEST_ADDRESS |staticValue |
|value |10 |int |
|tag |VALUE9TRANSACTION |string |

#In the default test, the latest sent index will be 52. The next milestone issued should be 53.
When a milestone is issued with index 51 and references:
|keys |values |type |
|transactions |previousTransaction |responseValue |

#Give the node time to solidify the milestone
And we wait "15" second/seconds

Given "getBalances" is called on "nodeA-m3" with:
|keys |values |type |
|addresses |TEST_EMPTY_ADDRESS |staticList |

Then the response for "getBalances" should return with:
|keys |values |type |
|balances |0 |int |


Scenario: Incomplete bundle doesnt affect ledger state
We want to ascertain that ledger state is always calculated correctly.
Even in the presence of incomplete bundles being approved.

Then "1" transaction is issued on "nodeA-m3" with:
|keys |values |type |
|address |TEST_ADDRESS |staticValue |
|value |0 |int |
|tag |ZERO9VALUE |string |

Then an incomplete bundle is generated referencing the previous transaction with:
|keys |values |type |
|address |TEST_EMPTY_ADDRESS |staticValue |
|tag |INCOMPLETE9TAG |string |

Then a transaction is issued referencing the previous transaction
|keys |values |type |
|seed |THE_BANK |staticList |
|address |TEST_ADDRESS |staticValue |
|value |11 |int |
|tag |VALUE9TRANSACTION |string |

#In the default test, the latest sent index will be 53. The next milestone issued should be 54.
When a milestone is issued with index 54 and references:
|keys |values |type |
|transactions |previousTransaction |responseValue |

#Give the node time to solidify the milestone
And we wait "10" second/seconds

Given "getBalances" is called on "nodeA-m3" with:
|keys |values |type |
|addresses |TEST_EMPTY_ADDRESS |staticList |

Then the response for "getBalances" should return with:
|keys |values |type |
|balances |0 |int |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you also move this to a different PR?
Less chances we will forget about this and it will also help @acha-bill ...

@@ -1,6 +1,6 @@
defaults: &transaction_tests_config_files
db: https://s3.eu-central-1.amazonaws.com/iotaledger-dbfiles/dev/Transactions_Tests_db.tar
db_checksum: 756237276479da4b01deaa0c1211ca65a4c8ec6f081452ea7e8153648c53bd67
db: https://s3.eu-central-1.amazonaws.com/iotaledger-dbfiles/dev/TransactionsTestsDb.tar
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the change to this DB documented?
Talk to @DyrellC about how to document this

@@ -173,7 +173,6 @@ Feature: Test API calls on Machine 1
Given "getBalances" is called on "nodeA-m4" with:
|keys |values |type |
|addresses |TEST_EMPTY_ADDRESS |staticList |
|threshold |100 |int |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should have been part of #1768
usually I would say to move this to another PR but I am getting soft

set_world_object(node, "firstDoubleSpend", [firstDoubleSpend.hash])

@step(r'a split bundle is generated referencing the previous transaction with:')
def create_split_bundle(step):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe this should be also part of #1807 ?

re_attached_bundle = Bundle.from_tryte_strings(re_attached_trytes)
return [original_bundle, re_attached_bundle]

def custom_attach(trytes, mwm):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

part of #1807?

bundle2.sign_inputs(KeyGenerator(seedFrom))
return [bundle1, bundle2]

def create_split_bundles(api, seedFrom, addressFrom, addressTo, addressRest, tag, value, reference):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

part of #1807?

@kwek20 kwek20 force-pushed the invalid-bundle-regression branch from bdd3138 to 3fda0e0 Compare March 25, 2020 19:02
@@ -0,0 +1,82 @@
from iota.crypto.signing import KeyGenerator

from iota import Iota, ProposedTransaction, Address, Bundle, TransactionHash, \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

argument_list = {'trunk_transaction': trunk1, 'branch_transaction': branch1,
'trytes': bundles[0].as_tryte_strings(), 'min_weight_magnitude': 14}
firstDoubleSpend = Transaction.from_tryte_string( transactions.attach_store_and_broadcast(api, argument_list).get('trytes')[0] )

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codacy Issue found: Trailing whitespace


argument_list = {'trunk_transaction': trunk2, 'branch_transaction': branch2,
'trytes': bundles[1].as_tryte_strings(), 'min_weight_magnitude': 14}
secondDoubleSpend = Transaction.from_tryte_string( transactions.attach_store_and_broadcast(api, argument_list).get('trytes')[0] )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create a regression test that checks balances in presence of invalid bundles
3 participants