Skip to content

INDY-2032: Fix phantom transactions in audit ledger #1151

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.

Sign up for GitHub

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

Merged
merged 10 commits into from
Apr 8, 2019
Merged

INDY-2032: Fix phantom transactions in audit ledger #1151

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
e118a3c
add test for a catchup for the new view
ashcherbakov Apr 4, 2019
a16851f
fix problem with phantom transactions during the view change
ashcherbakov Apr 5, 2019
6d94993
Merge branch 'master' of https://github.com/hyperledger/indy-plenum i…
ashcherbakov Apr 5, 2019
e3455a4
new tests and get rid of stashedOrderedRequest
ashcherbakov Apr 5, 2019
c14ee26
change logs
ashcherbakov Apr 5, 2019
012aa93
fix tests; process code review
ashcherbakov Apr 6, 2019
e8e35a3
fix tests
ashcherbakov Apr 6, 2019
d8b78b1
fix tests
ashcherbakov Apr 6, 2019
f9b0661
extend logs
ashcherbakov Apr 7, 2019
e4e9781
fix tests
ashcherbakov Apr 8, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 17 additions & 9 deletions plenum/server/node.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2670,7 +2670,23 @@ def processOrdered(self, ordered: Ordered):
ordered.stateRootHash,
ordered.txnRootHash))

self.executeBatch(ThreePcBatch.from_ordered(ordered),
three_pc_batch = ThreePcBatch.from_ordered(ordered)
if self.db_manager.ledgers[AUDIT_LEDGER_ID].uncommittedRootHash is None:
Copy link
Contributor

Choose a reason for hiding this comment

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

Could it happen so that uncommittedRootHash is not None but due to other batch in flight, so that we still need to manually apply transactions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think no, because here we always have apply-commit cycle, so that even if this is an Ordered msg came in between catchup rounds during view change, each Ordered will be re-applied and committed immediately, so that it always end up with empty uncommitted state if the audit ledger.

# if we order request during view change
# in between catchup rounds, then the 3PC batch will not be applied,
# since it was reverted before catchup started, and only COMMITs were
# processed in between catchup that led to this ORDERED msg
logger.info("{} applying stashed requests for batch {} {} of {} requests; state root {}; txn root {}"
.format(self.name,
three_pc_batch.view_no,
three_pc_batch.pp_seq_no,
len(three_pc_batch.valid_digests),
three_pc_batch.state_root,
three_pc_batch.txn_root))

self.apply_stashed_reqs(three_pc_batch)

self.executeBatch(three_pc_batch,
ordered.valid_reqIdr,
ordered.invalid_reqIdr,
ordered.auditTxnRootHash)
Expand Down Expand Up @@ -3298,14 +3314,6 @@ def updateSeqNoMap(self, committedTxns, ledger_id):

def commitAndSendReplies(self, three_pc_batch: ThreePcBatch) -> List:
logger.trace('{} going to commit and send replies to client'.format(self))

if self.db_manager.ledgers[AUDIT_LEDGER_ID].uncommittedRootHash is None:
# if we order request during view change
# in between catchup rounds, then the 3PC batch will not be applied,
# since it was reverted before catchup started, and only COMMITs were
# processed in between catchup that led to this ORDERED msg
self.apply_stashed_reqs(three_pc_batch)

reqHandler = self.get_req_handler(three_pc_batch.ledger_id)
committedTxns = reqHandler.commit(len(three_pc_batch.valid_digests),
three_pc_batch.state_root, three_pc_batch.txn_root,
Expand Down
8 changes: 6 additions & 2 deletions plenum/test/replica/stashing/test_unstash_after_catchup_in_view_change.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@

from plenum.common.constants import COMMIT, PREPREPARE, PREPARE, LEDGER_STATUS
from plenum.common.startable import Mode
from plenum.test.delayers import vcd_delay, msg_rep_delay, cDelay, cr_delay, lsDelay, cpDelay, cs_delay
from plenum.test.delayers import vcd_delay, msg_rep_delay, cDelay, cr_delay
from plenum.test.helper import waitForViewChange, sdk_send_random_and_check, assertExp, sdk_send_random_request, \
sdk_get_and_check_replies
from plenum.test.node_catchup.helper import waitNodeDataEquality
from plenum.test.node_catchup.helper import ensure_all_nodes_have_same_data
from plenum.test.node_request.helper import sdk_ensure_pool_functional
from plenum.test.stasher import delay_rules
from plenum.test.test_node import ensureElectionsDone
from stp_core.loop.eventually import eventually
Expand Down Expand Up @@ -107,6 +108,9 @@ def check_commits(commit_key):
for n in txnPoolNodeSet)
assert slow_node.catchup_rounds_without_txns == 1

ensure_all_nodes_have_same_data(looper, txnPoolNodeSet)
sdk_ensure_pool_functional(looper, txnPoolNodeSet, sdk_wallet_steward, sdk_pool_handle)


def _check_nodes_stashed(nodes, old_stashed, new_stashed):
for n in nodes:
Expand Down