Skip to content
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

Jump to bottom

Enable snapshot and finalizer commits integration #852

Merged
merged 13 commits into from
Apr 5, 2019
Merged

Enable snapshot and finalizer commits integration #852

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
743dd36
Enable snapshot and finalizer commits integration
frolosofsky Mar 26, 2019
fede51c
Add check to the snapshot
frolosofsky Apr 2, 2019
71eb8fc
post-rebase
frolosofsky Apr 2, 2019
4fbab20
Record votes
frolosofsky Apr 2, 2019
41b5ce8
Check VoteRecorder works during fast-sync
frolosofsky Apr 2, 2019
a1dfe94
Update best snapshot picking algorithm
frolosofsky Apr 3, 2019
19b358c
Slash em all
frolosofsky Apr 3, 2019
11b0166
Don't sign transaction
frolosofsky Apr 4, 2019
1da775e
cr followup
frolosofsky Apr 4, 2019
181f109
cr followup
frolosofsky Apr 4, 2019
d90c996
nits
frolosofsky Apr 4, 2019
120b1e2
return
frolosofsky Apr 5, 2019
d300fdb
slahsing
frolosofsky Apr 5, 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
2 changes: 1 addition & 1 deletion src/rpc/rawtransaction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1059,7 +1059,7 @@ UniValue extractvotefromsignature(const JSONRPCRequest &request) {
"1. \"signature\" (string).\n"
"Result:\n"
"{\n"
" \"validator_ddress\": xxxx (string) the validator address\n"
" \"validator_address\": xxxx (string) the validator address\n"
" \"target_hash\": xxxx\n (string) the target hash"
" \"source_epoch\": xxxx (numeric) the source epoch\n"
" \"target_epoch\": xxxx (numeric) the target epoch\n"
Expand Down
14 changes: 9 additions & 5 deletions test/functional/feature_snapshot_finalization.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
connect_nodes,
disconnect_nodes,
sync_blocks,
sync_chain,
wait_until,
)

Expand Down Expand Up @@ -68,7 +67,7 @@ def run_test(self):
disconnect_nodes(p, v.index)

self.log.info("Generate few epochs")
votes = self.generate_epoch(epoch_length=5, proposer=p, finalizer=v, count=2)
votes = self.generate_epoch(proposer=p, finalizer=v, count=2)
assert(len(votes) != 0)

assert_equal(p.getblockcount(), 32)
Expand All @@ -87,7 +86,7 @@ def run_test(self):
'validators': 1})

self.log.info("Generate next epoch")
votes += self.generate_epoch(epoch_length=5, proposer=p, finalizer=v, count=1)
votes += self.generate_epoch(proposer=p, finalizer=v, count=1)

assert_equal(p.getblockcount(), 37)
assert_finalizationstate(p, {'currentEpoch': 8,
Expand All @@ -101,10 +100,15 @@ def run_test(self):
'lastFinalizedEpoch': 6,
'validators': 1})

self.log.info("Check slashig condition");
self.log.info("Check slashig condition")
# Create new vote with input=votes[-1] which attepts to make a double vote
frolosofsky marked this conversation as resolved.
Show resolved Hide resolved
# To detect double vote, it's enough to two votes were:
frolosofsky marked this conversation as resolved.
Show resolved Hide resolved
scravy marked this conversation as resolved.
Show resolved Hide resolved
# 1. from same validator
# 2. with same source epoch
# 3. differet target epochs.
scravy marked this conversation as resolved.
Show resolved Hide resolved
# So that make target epoch different.
vote = s.extractvotefromsignature(bytes_to_hex_str(votes[0].vin[0].scriptSig))
vote['target_epoch'] = vote['target_epoch'] + 1;
vote['target_epoch'] = vote['target_epoch'] + 1
prev_tx = s.decoderawtransaction(ToHex(votes[-1]))
vtx = v.createvotetransaction(vote, prev_tx['txid'])
vtx = v.signrawtransaction(vtx)
Expand Down
4 changes: 2 additions & 2 deletions test/functional/p2p_snapshot.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,7 @@ def test_invalid_snapshot(self):
3. node - the node which syncs the snapshot
4. broken_p2p - mini node that claims has the best snapshot but it's broken
5. valid_p2p - mini node that sends a valid snapshot
6. not_finalized_p2p - mini node that claims has the best snapshot but it's not finalied
6. not_finalized_p2p - mini node that claims has the best snapshot but it's not finalized
"""

snap_node = self.nodes[4]
Expand Down Expand Up @@ -526,7 +526,7 @@ def test_invalid_snapshot(self):
valid_p2p.return_parent_block = True
valid_p2p.on_getsnapshot(valid_p2p.last_getsnapshot_message)

# node doesn't request not finalied snapshot
# node doesn't request not finalized snapshot
assert_equal(not_finalized_p2p.snapshot_header_requested, True)
assert_equal(not_finalized_p2p.snapshot_chunk1_requested, False)

Expand Down
9 changes: 6 additions & 3 deletions test/functional/test_framework/test_framework.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,17 +446,20 @@ def generate_sync(self, generator_node, nblocks=1, nodes=None):
return generated_blocks

@classmethod
def generate_epoch(cls, epoch_length, proposer, finalizer, count=1):
def generate_epoch(cls, proposer, finalizer, count=1):
Copy link
Member

@kostyantyn kostyantyn Apr 4, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Would suggest keeping this function in the current test feature_snapshot_finalization.py because it's not generic enough to use in multiple scenarios and then work on it separately.

  1. it won't work correctly if we are at height=0 as we will generate 4 blocks (we will be at the checkpoint) and node can't vote at a checkpoint
  2. It can be useful to provide the list of finalizers and then everyone must vote. I see a few examples then when this function can be used.
  3. I'd like to control where the votes are included. In most of tests, we vote after the 1st block, here we vote at checkpoint-1.
  4. Would expect to return votes of my finalizer(s) but not all.

Copy link
Member Author

Choose a reason for hiding this comment

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

It seems to be used in #883. I used similar algorithms in couple of tests already.

it won't work correctly if we are at height=0 as we will generate 4 blocks (we will be at the checkpoint) and node can't vote at a checkpoint
It can be useful to provide the list of finalizers and then everyone must vote. I see a few examples then when this function can be used.

Thanks, I will fix it when move this function al long as wait_for_vote_and_disconnect to the util.py. Let's focus on this in different PR.

I'd like to control where the votes are included. In most of tests, we vote after the 1st block, here we vote at checkpoint-1.

If you like to control, you probably write your code explicitly. This function is a variation of generatetoaddress(X * EPOCH_LENGTH) which ensures finalizer is in time to vote for every epoch.

"""
Generate count epochs and collect votes.
Generate `count` epochs and collect votes.
"""
epoch_length = proposer.getfinalizationconfig()['epochLength']
assert(epoch_length > 1)
frolosofsky marked this conversation as resolved.
Show resolved Hide resolved
votes=[]
for _ in range(count):
proposer.generatetoaddress(epoch_length - 1, proposer.getnewaddress('', 'bech32'))
cls.wait_for_vote_and_disconnect(finalizer, proposer)
for tx in proposer.getrawmempool():
votes.append(FromHex(CTransaction(), proposer.getrawtransaction(tx)))
tx = FromHex(CTransaction(), proposer.getrawtransaction(tx))
if tx.get_type() == TxType.VOTE:
votes.append(tx)
proposer.generatetoaddress(1, proposer.getnewaddress('', 'bech32'))
return votes

Expand Down