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.

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

test: fixes and improvements for feature_llmq_simplepose.py #6287

Merged
merged 7 commits into from
Sep 26, 2024

Conversation

UdjinM6
Copy link

@UdjinM6 UdjinM6 commented Sep 25, 2024

Issue being fixed or feature implemented

Fix some general mistakes and also connect_nodes related issues discovered while debugging #6276. Add some logging to make debugging a bit easier.

NOTE: builds on top of #6278 to avoid conflicts, will rebase done

What was done?

pls see individual commits

How Has This Been Tested?

run tests

Breaking Changes

n/a

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation
  • I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

@UdjinM6 UdjinM6 added this to the 21.2 milestone Sep 25, 2024
@PastaPastaPasta
Copy link
Member

6278 merged

@UdjinM6 UdjinM6 marked this pull request as ready for review September 25, 2024 14:39
Copy link
Collaborator

@kwvg kwvg left a comment

Choose a reason for hiding this comment

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

LGTM, ACK a656d2f

@knst
Copy link
Collaborator

knst commented Sep 26, 2024

just some semi-relevant feedback on PR

CI: only one failure out of 5 (for linux64-multiprocess): https://gitlab.com/dashpay/dash/-/jobs/7917711153

feature_llmq_simplepose.py failed at attempt 1/3, Duration: 71 s
37/265 - feature_llmq_simplepose.py passed, Duration: 225 s

I run this version on my laptop in many threads and observed various different failures:

Beside 40 successful runs I noticed these failures,

  • 3 times for check_banned:
2024-09-26T07:16:25.800000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/home/knst/projects/dash-reviews/test/functional/test_framework/test_framework.py", line 160, in main
    self.run_test()
  File "/home/knst/projects/dash-reviews/test/functional/feature_llmq_simplepose.py", line 34, in run_test
    self.test_banning(self.isolate_mn, 2)
  File "/home/knst/projects/dash-reviews/test/functional/feature_llmq_simplepose.py", line 191, in test_banning
    assert self.check_banned(mn)
AssertionError
  • once not punished no banned:
2024-09-26T07:23:54.730000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/home/knst/projects/dash-reviews/test/functional/test_framework/test_framework.py", line 160, in main
    self.run_test()
  File "/home/knst/projects/dash-reviews/test/functional/feature_llmq_simplepose.py", line 31, in run_test
    self.test_no_banning()
  File "/home/knst/projects/dash-reviews/test/functional/feature_llmq_simplepose.py", line 98, in test_no_banning
    assert not self.check_punished(mn) and not self.check_banned(mn)
AssertionError
  • once no quorum:
  File "/home/knst/projects/dash-reviews/test/functional/feature_llmq_simplepose.py", line 48, in run_test
    self.test_banning(self.close_mn_port)
  File "/home/knst/projects/dash-reviews/test/functional/feature_llmq_simplepose.py", line 189, in test_banning
    self.mine_quorum_less_checks(expected_contributors - 1, mninfos_online)
  File "/home/knst/projects/dash-reviews/test/functional/feature_llmq_simplepose.py", line 148, in mine_quorum_less_checks
    self.wait_for_quorum_list(q, nodes)
  File "/home/knst/projects/dash-reviews/test/functional/test_framework/test_framework.py", line 1779, in wait_for_quorum_list
    wait_until_helper(wait_func, timeout=timeout, sleep=sleep)
  File "/home/knst/projects/dash-reviews/test/functional/test_framework/util.py", line 262, in wait_until_helper
    raise AssertionError("Predicate {} not true after {} seconds".format(predicate_source, timeout))
AssertionError: Predicate ''''
        def wait_func():
            self.log.info("quorums: " + str(self.nodes[0].quorum("list")))
            if quorum_hash in self.nodes[0].quorum("list")[llmq_type_name]:
                return True
            self.bump_mocktime(sleep, nodes=nodes)
            self.nodes[0].generate(1)
            self.sync_blocks(nodes)
            return False
''' not true after 15.0 seconds
  • once failed at contribution stage for quorum:
2024-09-26T07:23:49.843000Z TestFramework (INFO): Waiting for phase 2 (contribute)
2024-09-26T07:24:19.901000Z TestFramework.utils (ERROR): wait_until() failed. Predicate: ''''
        def check_dkg_session():
            member_count = 0
            for mn in mninfos:
                s = mn.node.quorum("dkgstatus")["session"]
                for qs in s:
                    if qs["llmqType"] != llmq_type_name:
                        continue
                    qstatus = qs["status"]
                    if qstatus["quorumHash"] != quorum_hash:
                        continue
                    if qstatus["phase"] != phase:
                        return False
                    if check_received_messages is not None:
                        if qstatus[check_received_messages] < check_received_messages_count:
                            return False
                    member_count += 1
                    break
            return member_count >= expected_member_count
'''
2024-09-26T07:24:19.902000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/home/knst/projects/dash-reviews/test/functional/test_framework/test_framework.py", line 160, in main
    self.run_test()
  File "/home/knst/projects/dash-reviews/test/functional/feature_llmq_simplepose.py", line 31, in run_test
    self.test_no_banning()
  File "/home/knst/projects/dash-reviews/test/functional/feature_llmq_simplepose.py", line 96, in test_no_banning
    self.mine_quorum(expected_connections=expected_connections)
  File "/home/knst/projects/dash-reviews/test/functional/test_framework/test_framework.py", line 1839, in mine_quorum
    self.wait_for_quorum_phase(q, 2, expected_members, "receivedContributions", expected_contributions, mninfos_online, llmq_type_name=llmq_type_name)
  File "/home/knst/projects/dash-reviews/test/functional/test_framework/test_framework.py", line 1747, in wait_for_quorum_phase
    wait_until_helper(check_dkg_session, timeout=timeout, sleep=sleep)
  File "/home/knst/projects/dash-reviews/test/functional/test_framework/util.py", line 262, in wait_until_helper
    raise AssertionError("Predicate {} not true after {} seconds".format(predicate_source, timeout))

Copy link
Collaborator

@knst knst left a comment

Choose a reason for hiding this comment

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

ACK a656d2f

Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

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

utACK a656d2f

@knst knst merged commit 67b5d86 into dashpay:develop Sep 26, 2024
23 of 38 checks passed
PastaPastaPasta added a commit that referenced this pull request Oct 4, 2024
, bitcoin#23774, bitcoin#25443, bitcoin#26138, bitcoin#26854, bitcoin#27128, bitcoin#27761, bitcoin#27863, bitcoin#28287, bitcoin#30118, partial bitcoin#22778 (auxiliary backports: part 16)

e458adb merge bitcoin#30118: improve robustness of connect_nodes() (UdjinM6)
ac94de2 merge bitcoin#28287: add `sendmsgtopeer` rpc and a test for net-level deadlock situation (Kittywhiskers Van Gogh)
d1fce0b fix: ensure that deadlocks are actually resolved (Kittywhiskers Van Gogh)
19e7bf6 merge bitcoin#27863: do not break when addr is not from a distinct network group (Kittywhiskers Van Gogh)
1adb9a2 merge bitcoin#27761: Log addresses of stalling peers (Kittywhiskers Van Gogh)
2854a6a merge bitcoin#27128: fix intermittent issue in `p2p_disconnect_ban` (Kittywhiskers Van Gogh)
d4b0fae merge bitcoin#26854: Fix intermittent timeout in p2p_permissions.py (Kittywhiskers Van Gogh)
892e329 merge bitcoin#26138: Avoid race in disconnect_nodes helper (Kittywhiskers Van Gogh)
d6ce037 merge bitcoin#25443: Fail if connect_nodes fails (Kittywhiskers Van Gogh)
60b5392 partial bitcoin#22778: Reduce resource usage for inbound block-relay-only connections (Kittywhiskers Van Gogh)
85c4aef merge bitcoin#23774: Add missing assert_equal import to p2p_add_connections.py (Kittywhiskers Van Gogh)
0354417 merge bitcoin#22777: don't request tx relay on feeler connections (Kittywhiskers Van Gogh)
7229eb0 merge bitcoin#23042: Avoid logging AlreadyHaveTx when disconnecting misbehaving peer (Kittywhiskers Van Gogh)
05395ff merge bitcoin#22817: Avoid race after connect_nodes (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * Depends on #6286

  * Depends on #6287

  * Depends on #6289

  * When backporting [bitcoin#28287](bitcoin#28287), `p2p_net_deadlock.py` relies on the function, `random_bytes()`, that is introduced in [bitcoin#25625](bitcoin#25625). Backporting [bitcoin#25625](bitcoin#25625) would attract changes outside the scope of this PR.

    In the interest of brevity, the changes that introduce `random_bytes()` have been included in [bitcoin#28287](bitcoin#28287) instead.

  ## Breaking Changes

  None expected.

  ## Checklist:

  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [x] I have made corresponding changes to the documentation **(note: N/A)**
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    utACK e458adb
  PastaPastaPasta:
    utACK e458adb

Tree-SHA512: 48494004dddecb31c53f5e19ab0114b92ed7b4381c7977800fd49b7403222badbfdcfe46241e854f5b086c6f54a35f6483f91c6f047b7ac9b1e88e35bb32ad02
@UdjinM6 UdjinM6 modified the milestones: 21.2, 22 Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants