-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ST-525 Simple chain tests #220
Conversation
Signed-off-by: Nikita Khateev <nikita.khateev@dsr-corporation.com>
Signed-off-by: Nikita Khateev <nikita.khateev@dsr-corporation.com>
from stp_core.loop.eventually import eventually | ||
|
||
|
||
def test_revert_fees_with_xfer(nodeSetWithIntegratedTokenPlugin, xfer_mint_tokens, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a comment what this test does.
from plenum.test.test_node import ensureElectionsDone | ||
from plenum.test.view_change.helper import ensure_view_change | ||
from stp_core.loop.eventually import eventually | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make sure that these tests don't do the same as tests in #217
from stp_core.loop.eventually import eventually | ||
|
||
|
||
def test_revert_fees_with_xfer(nodeSetWithIntegratedTokenPlugin, xfer_mint_tokens, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a better name for the test can be test_revert_during_view_change_all_nodes_xfer_with_fees
assert utxos[OUTPUTS][0][SEQNO] == seq_no+1 | ||
|
||
|
||
def test_revert_fees_with_nym(nodeSetWithIntegratedTokenPlugin, xfer_mint_tokens, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a better name for the test can be test_revert_during_view_change_all_nodes_nym_with_fees
with delay_rules(node_set, cDelay()): | ||
# should be changed for auth rule | ||
helpers.general.set_fees_without_waiting({XFER_PUBLIC: 2}) | ||
looper.runFor(waits.expectedPrePrepareTime(len(node_set))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In order to have deterministic test, please wait for PrePrepare being applied instead.
You can use, for example,
looper.run(eventually(check_prepare_certificate, node_set, initial_pp_seq_no+1))
for n in nodes: | ||
looper.run(eventually(lambda: assertExp(n.mode == Mode.participating))) | ||
for n in nodes: | ||
looper.run(eventually(check_state, n, True, retryWait=0.2, timeout=15)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is already checked by ensure_all_nodes_have_same_data
|
||
utxos = helpers.general.get_utxo_addresses(addresses[3:]) | ||
|
||
assert utxos[0][0]["amount"] == 988 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why 988? Where do we set Fees? It would be great to set them explicitly in the test.
Signed-off-by: Nikita Khateev <nikita.khateev@dsr-corporation.com>
Signed-off-by: Nikita Khateev <nikita.khateev@dsr-corporation.com>
Signed-off-by: Nikita Khateev <nikita.khateev@dsr-corporation.com>
Signed-off-by: Nikita Khateev <nikita.khateev@dsr-corporation.com>
old_max_size = tconf.Max3PCBatchSize | ||
old_time = tconf.Max3PCBatchWait | ||
tconf.Max3PCBatchSize = 2 | ||
tconf.Max3PCBatchWait = 5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is it 5
and not 1000
? If we want to have a deterministic test to make sure that every batch has exactly 2 txns, then we must set Max3PCBatchSize=2
and Max3PCBatchWait ->infinity
.
Signed-off-by: Nikita Khateev <nikita.khateev@dsr-corporation.com>
Signed-off-by: Nikita Khateev <nikita.khateev@dsr-corporation.com>
# should be changed for auth rule | ||
helpers.general.set_fees_without_waiting({ATTRIB: 4}) | ||
looper.run(eventually(functools.partial(check_batch_ordered, _old_pp_seq_no, nodeSetWithIntegratedTokenPlugin))) | ||
ensure_view_change(looper, nodes) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now, we don't revert SET_FEES txn, and setting fees to 4 ({ATTRIB: 4}) should be reverted to {ATTRIB: 3}. Fees attribute in StaticFeeReqHandler class, which used into validation txns with fees. This map will be changed on updateState, but not reverted. I think, that we should change 'fees' attribute to property, which should use state. But for now, maybe we should create a test, which should failed now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added such test
Signed-off-by: Nikita Khateev <nikita.khateev@dsr-corporation.com>
Signed-off-by: Nikita Khateev <nikita.khateev@dsr-corporation.com>
No description provided.