Page MenuHomePhabricator
Differential D9498

[avalanche] Reply to proof invs with getdata requests
AbandonedPublic
Actions

Authored by Fabien on May 11 2021, 14:43.

Details

Reviewers
deadalnix
Group Reviewers
Restricted Project
Summary

This a very basic implementation of the proof download feature via a
getdata message. For now this has zero defense mechanism so it is hidden
behind a flag which disables it by default.

Depends on D9499 for passing the tests.

Test Plan
./test/functional/test_runner.py abc_p2p_avalanche_inventory.py

Diff Detail

Repository
rABC Bitcoin ABC
Branch
avalanche_answer_inv_getdata
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 15680
Build 31264: Build Diffbuild-debug · build-without-wallet · build-diff · lint-circular-dependencies · build-clang · build-clang-tidy
Build 31263: arc lint + arc unit

Event Timeline

Fabien created this revision.May 11 2021, 14:43
Herald added a reviewer: Restricted Project. · View Herald TranscriptMay 11 2021, 14:43
Fabien requested review of this revision.May 11 2021, 14:43
Fabien updated this revision to Diff 28416.May 11 2021, 14:45

Move the log to the lambda as well

Harbormaster completed remote builds in B15680: Diff 28415.May 11 2021, 14:53
teamcity added a comment.May 11 2021, 14:54
Build Bitcoin ABC Diffs / Diff Testing (build-debug) failed.

Failed tests logs:

====== Bitcoin ABC functional tests: abc_p2p_proof_inventory.py ======

------- Stdout: -------
2021-05-11T14:52:34.480000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/build-debug/test/tmp/test_runner_₿₵_  _20210511_144836/abc_p2p_proof_inventory_617
2021-05-11T14:52:35.068000Z TestFramework (INFO): Test that we request proofs from all our peers, eventually
2021-05-11T14:52:36.228000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/work/test/functional/test_framework/test_framework.py", line 126, in main
    self.run_test()
  File "/work/test/functional/abc_p2p_proof_inventory.py", line 53, in run_test
    self.test_proof_requests()
  File "/work/test/functional/abc_p2p_proof_inventory.py", line 43, in test_proof_requests
    p.send_and_ping(msg)
  File "/work/test/functional/test_framework/mininode.py", line 567, in send_and_ping
    self.sync_with_ping(timeout=timeout)
  File "/work/test/functional/test_framework/mininode.py", line 578, in sync_with_ping
    self.wait_until(test_function, timeout=timeout)
  File "/work/test/functional/test_framework/mininode.py", line 470, in wait_until
    timeout_factor=self.timeout_factor)
  File "/work/test/functional/test_framework/util.py", line 261, in wait_until
    if predicate():
  File "/work/test/functional/test_framework/mininode.py", line 574, in test_function
    assert self.is_connected
AssertionError
2021-05-11T14:52:36.279000Z TestFramework (INFO): Stopping nodes
------- Stderr: -------
Traceback (most recent call last):
  File "/work/test/functional/test_framework/authproxy.py", line 116, in _request
    self.__conn.request(method, path, postdata, headers)
  File "/usr/lib/python3.7/http/client.py", line 1260, in request
    self._send_request(method, url, body, headers, encode_chunked)
  File "/usr/lib/python3.7/http/client.py", line 1306, in _send_request
    self.endheaders(body, encode_chunked=encode_chunked)
  File "/usr/lib/python3.7/http/client.py", line 1255, in endheaders
    self._send_output(message_body, encode_chunked=encode_chunked)
  File "/usr/lib/python3.7/http/client.py", line 1069, in _send_output
    self.send(chunk)
  File "/usr/lib/python3.7/http/client.py", line 991, in send
    self.sock.sendall(data)
BrokenPipeError: [Errno 32] Broken pipe

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/work/test/functional/abc_p2p_proof_inventory.py", line 57, in <module>
    ProofInventoryTest().main()
  File "/work/test/functional/test_framework/test_framework.py", line 146, in main
    exit_code = self.shutdown()
  File "/work/test/functional/test_framework/test_framework.py", line 272, in shutdown
    self.stop_nodes()
  File "/work/test/functional/test_framework/test_framework.py", line 500, in stop_nodes
    node.stop_node(wait=wait, wait_until_stopped=False)
  File "/work/test/functional/test_framework/test_node.py", line 412, in stop_node
    self.stop(wait=wait)
  File "/work/test/functional/test_framework/coverage.py", line 48, in __call__
    return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
  File "/work/test/functional/test_framework/authproxy.py", line 157, in __call__
    'POST', self.__url.path, postdata.encode('utf-8'))
  File "/work/test/functional/test_framework/authproxy.py", line 122, in _request
    self.__conn.request(method, path, postdata, headers)
  File "/usr/lib/python3.7/http/client.py", line 1260, in request
    self._send_request(method, url, body, headers, encode_chunked)
  File "/usr/lib/python3.7/http/client.py", line 1306, in _send_request
    self.endheaders(body, encode_chunked=encode_chunked)
  File "/usr/lib/python3.7/http/client.py", line 1255, in endheaders
    self._send_output(message_body, encode_chunked=encode_chunked)
  File "/usr/lib/python3.7/http/client.py", line 1030, in _send_output
    self.send(msg)
  File "/usr/lib/python3.7/http/client.py", line 970, in send
    self.connect()
  File "/usr/lib/python3.7/http/client.py", line 942, in connect
    (self.host,self.port), self.timeout, self.source_address)
  File "/usr/lib/python3.7/socket.py", line 727, in create_connection
    raise err
  File "/usr/lib/python3.7/socket.py", line 716, in create_connection
    sock.connect(sa)
ConnectionRefusedError: [Errno 111] Connection refused

Each failure log is accessible here:
Bitcoin ABC functional tests: abc_p2p_proof_inventory.py

Harbormaster completed remote builds in B15681: Diff 28416.May 11 2021, 14:58
teamcity added a comment.May 11 2021, 14:58
Build Bitcoin ABC Diffs / Diff Testing (build-debug) failed.

Failed tests logs:

====== Bitcoin ABC functional tests: abc_p2p_proof_inventory.py ======

------- Stdout: -------
2021-05-11T14:58:16.037000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/build-debug/test/tmp/test_runner_₿₵_  _20210511_145429/abc_p2p_proof_inventory_437
2021-05-11T14:58:16.619000Z TestFramework (INFO): Test that we request proofs from all our peers, eventually
2021-05-11T14:58:17.907000Z TestFramework.mininode (WARNING): Connection lost to 127.0.0.1:20257 due to [Errno 104] Connection reset by peer
2021-05-11T14:58:17.936000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/work/test/functional/test_framework/test_framework.py", line 126, in main
    self.run_test()
  File "/work/test/functional/abc_p2p_proof_inventory.py", line 53, in run_test
    self.test_proof_requests()
  File "/work/test/functional/abc_p2p_proof_inventory.py", line 43, in test_proof_requests
    p.send_and_ping(msg)
  File "/work/test/functional/test_framework/mininode.py", line 567, in send_and_ping
    self.sync_with_ping(timeout=timeout)
  File "/work/test/functional/test_framework/mininode.py", line 578, in sync_with_ping
    self.wait_until(test_function, timeout=timeout)
  File "/work/test/functional/test_framework/mininode.py", line 470, in wait_until
    timeout_factor=self.timeout_factor)
  File "/work/test/functional/test_framework/util.py", line 261, in wait_until
    if predicate():
  File "/work/test/functional/test_framework/mininode.py", line 574, in test_function
    assert self.is_connected
AssertionError
2021-05-11T14:58:17.988000Z TestFramework (INFO): Stopping nodes
[node 0] Cleaning up leftover process
------- Stderr: -------
Traceback (most recent call last):
  File "/work/test/functional/test_framework/authproxy.py", line 116, in _request
    self.__conn.request(method, path, postdata, headers)
  File "/usr/lib/python3.7/http/client.py", line 1260, in request
    self._send_request(method, url, body, headers, encode_chunked)
  File "/usr/lib/python3.7/http/client.py", line 1306, in _send_request
    self.endheaders(body, encode_chunked=encode_chunked)
  File "/usr/lib/python3.7/http/client.py", line 1255, in endheaders
    self._send_output(message_body, encode_chunked=encode_chunked)
  File "/usr/lib/python3.7/http/client.py", line 1069, in _send_output
    self.send(chunk)
  File "/usr/lib/python3.7/http/client.py", line 991, in send
    self.sock.sendall(data)
BrokenPipeError: [Errno 32] Broken pipe

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/work/test/functional/abc_p2p_proof_inventory.py", line 57, in <module>
    ProofInventoryTest().main()
  File "/work/test/functional/test_framework/test_framework.py", line 146, in main
    exit_code = self.shutdown()
  File "/work/test/functional/test_framework/test_framework.py", line 272, in shutdown
    self.stop_nodes()
  File "/work/test/functional/test_framework/test_framework.py", line 500, in stop_nodes
    node.stop_node(wait=wait, wait_until_stopped=False)
  File "/work/test/functional/test_framework/test_node.py", line 412, in stop_node
    self.stop(wait=wait)
  File "/work/test/functional/test_framework/coverage.py", line 48, in __call__
    return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
  File "/work/test/functional/test_framework/authproxy.py", line 157, in __call__
    'POST', self.__url.path, postdata.encode('utf-8'))
  File "/work/test/functional/test_framework/authproxy.py", line 122, in _request
    self.__conn.request(method, path, postdata, headers)
  File "/usr/lib/python3.7/http/client.py", line 1260, in request
    self._send_request(method, url, body, headers, encode_chunked)
  File "/usr/lib/python3.7/http/client.py", line 1306, in _send_request
    self.endheaders(body, encode_chunked=encode_chunked)
  File "/usr/lib/python3.7/http/client.py", line 1255, in endheaders
    self._send_output(message_body, encode_chunked=encode_chunked)
  File "/usr/lib/python3.7/http/client.py", line 1030, in _send_output
    self.send(msg)
  File "/usr/lib/python3.7/http/client.py", line 970, in send
    self.connect()
  File "/usr/lib/python3.7/http/client.py", line 942, in connect
    (self.host,self.port), self.timeout, self.source_address)
  File "/usr/lib/python3.7/socket.py", line 727, in create_connection
    raise err
  File "/usr/lib/python3.7/socket.py", line 716, in create_connection
    sock.connect(sa)
ConnectionRefusedError: [Errno 111] Connection refused

Each failure log is accessible here:
Bitcoin ABC functional tests: abc_p2p_proof_inventory.py

teamcity added a comment.May 11 2021, 15:02
Build Bitcoin ABC Diffs / Diff Testing (build-diff) failed.

Failed tests logs:

====== Bitcoin ABC functional tests: abc_mining_basic.py ======

------- Stdout: -------
2021-05-11T14:57:43.435000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20210511_145444/abc_mining_basic_342
2021-05-11T14:57:43.911000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/work/test/functional/test_framework/test_framework.py", line 126, in main
    self.run_test()
  File "/work/test/functional/abc_mining_basic.py", line 146, in run_test
    self.run_for_node(self.nodes[1], MINER_FUND_LEGACY_ADDR)
  File "/work/test/functional/abc_mining_basic.py", line 69, in run_for_node
    'minimumvalue': 0,
  File "/work/test/functional/abc_mining_basic.py", line 58, in assert_getblocktemplate
    assert_equal(blockTemplate[key], value)
  File "/work/test/functional/test_framework/util.py", line 60, in assert_equal
    for arg in (thing1, thing2) + args)))
AssertionError: not({'minerfund': {'addresses': ['2MviGxxFciGeWTgkUgYgjqehWt18c4ZsShd'], 'minimumvalue': 200000000}} == {'minerfund': {'addresses': [], 'minimumvalue': 0}})
2021-05-11T14:57:43.962000Z TestFramework (INFO): Stopping nodes
2021-05-11T14:57:44.164000Z TestFramework (WARNING): Not cleaning up dir /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20210511_145444/abc_mining_basic_342
2021-05-11T14:57:44.164000Z TestFramework (ERROR): Test failed. Test logging available at /work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20210511_145444/abc_mining_basic_342/test_framework.log
2021-05-11T14:57:44.164000Z TestFramework (ERROR): 
2021-05-11T14:57:44.164000Z TestFramework (ERROR): Hint: Call /work/test/functional/combine_logs.py '/work/abc-ci-builds/build-diff/test/tmp/test_runner_₿₵_  _20210511_145444/abc_mining_basic_342' to consolidate all logs
2021-05-11T14:57:44.164000Z TestFramework (ERROR): 
2021-05-11T14:57:44.164000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2021-05-11T14:57:44.164000Z TestFramework (ERROR): https://github.com/Bitcoin-ABC/bitcoin-abc/issues
2021-05-11T14:57:44.164000Z TestFramework (ERROR):

Each failure log is accessible here:
Bitcoin ABC functional tests: abc_mining_basic.py

Fabien planned changes to this revision.May 11 2021, 15:08
Fabien updated this revision to Diff 28418.May 11 2021, 19:21

Rebase on top of D9499 to fix the lock order issue

Fabien edited the summary of this revision. (Show Details)May 11 2021, 19:21
Fabien added a parent revision: D9499: [avalanche] Reverse cs_main and cs_peerManager lock order.
Harbormaster completed remote builds in B15683: Diff 28418.May 11 2021, 19:40
Mengerian added a subscriber: Mengerian.May 11 2021, 22:42
Mengerian added inline comments.
src/init.cpp
1240 ↗(On Diff #28418)

Question/suggestion: Is it likely we'll want to put other experimental stuff behind this flag in the future? If so, maybe it would it make sense to use a more generic name like "-enableavaexperimental" or similar?

In any case starting it with "-enable" seems good, as it's a flag that turns on a certain behavior.

deadalnix requested changes to this revision.May 11 2021, 22:48
deadalnix added a subscriber: deadalnix.
deadalnix added inline comments.
src/net_processing.cpp
522 ↗(On Diff #28418)

Why is a mutex needed here?

1788 ↗(On Diff #28418)
!= nullptr
3183 ↗(On Diff #28418)

Once again, in term of priority, this ought to come before txns.

5589 ↗(On Diff #28418)

dito

This revision now requires changes to proceed.May 11 2021, 22:48
Fabien updated this revision to Diff 28422.May 12 2021, 08:07

Rename the flag, remove the unecessary mutex, make the bool check explicit, move proof between block and tx

Harbormaster completed remote builds in B15685: Diff 28422.May 12 2021, 08:40
deadalnix added inline comments.May 12 2021, 11:40
src/net_processing.cpp
1786 ↗(On Diff #28422)
return g_avalanche && g_avalanche->getProof(proofid) != nullptr;
3177 ↗(On Diff #28422)

continue, or maybe make this whole thing a switch statement.

Fabien updated this revision to Diff 28424.May 12 2021, 11:54

Add missing continue, update nullptr position

Harbormaster completed remote builds in B15686: Diff 28424.May 12 2021, 12:28
teamcity added a comment.May 12 2021, 12:28
Build Bitcoin ABC Diffs / Diff Testing (build-debug) failed.

Failed tests logs:

====== Bitcoin ABC functional tests: abc_p2p_proof_inventory.py ======

------- Stdout: -------
2021-05-12T12:26:27.557000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/build-debug/test/tmp/test_runner_₿₵_  _20210512_122228/abc_p2p_proof_inventory_220
2021-05-12T12:26:28.116000Z TestFramework (INFO): Test that we request proofs from all our peers, eventually
2021-05-12T12:26:29.370000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/work/test/functional/test_framework/test_framework.py", line 126, in main
    self.run_test()
  File "/work/test/functional/abc_p2p_proof_inventory.py", line 53, in run_test
    self.test_proof_requests()
  File "/work/test/functional/abc_p2p_proof_inventory.py", line 43, in test_proof_requests
    p.send_and_ping(msg)
  File "/work/test/functional/test_framework/mininode.py", line 567, in send_and_ping
    self.sync_with_ping(timeout=timeout)
  File "/work/test/functional/test_framework/mininode.py", line 578, in sync_with_ping
    self.wait_until(test_function, timeout=timeout)
  File "/work/test/functional/test_framework/mininode.py", line 470, in wait_until
    timeout_factor=self.timeout_factor)
  File "/work/test/functional/test_framework/util.py", line 261, in wait_until
    if predicate():
  File "/work/test/functional/test_framework/mininode.py", line 574, in test_function
    assert self.is_connected
AssertionError
2021-05-12T12:26:29.421000Z TestFramework (INFO): Stopping nodes
------- Stderr: -------
Traceback (most recent call last):
  File "/work/test/functional/test_framework/authproxy.py", line 116, in _request
    self.__conn.request(method, path, postdata, headers)
  File "/usr/lib/python3.7/http/client.py", line 1260, in request
    self._send_request(method, url, body, headers, encode_chunked)
  File "/usr/lib/python3.7/http/client.py", line 1306, in _send_request
    self.endheaders(body, encode_chunked=encode_chunked)
  File "/usr/lib/python3.7/http/client.py", line 1255, in endheaders
    self._send_output(message_body, encode_chunked=encode_chunked)
  File "/usr/lib/python3.7/http/client.py", line 1069, in _send_output
    self.send(chunk)
  File "/usr/lib/python3.7/http/client.py", line 991, in send
    self.sock.sendall(data)
BrokenPipeError: [Errno 32] Broken pipe

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/work/test/functional/abc_p2p_proof_inventory.py", line 57, in <module>
    ProofInventoryTest().main()
  File "/work/test/functional/test_framework/test_framework.py", line 146, in main
    exit_code = self.shutdown()
  File "/work/test/functional/test_framework/test_framework.py", line 272, in shutdown
    self.stop_nodes()
  File "/work/test/functional/test_framework/test_framework.py", line 500, in stop_nodes
    node.stop_node(wait=wait, wait_until_stopped=False)
  File "/work/test/functional/test_framework/test_node.py", line 412, in stop_node
    self.stop(wait=wait)
  File "/work/test/functional/test_framework/coverage.py", line 48, in __call__
    return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
  File "/work/test/functional/test_framework/authproxy.py", line 157, in __call__
    'POST', self.__url.path, postdata.encode('utf-8'))
  File "/work/test/functional/test_framework/authproxy.py", line 122, in _request
    self.__conn.request(method, path, postdata, headers)
  File "/usr/lib/python3.7/http/client.py", line 1260, in request
    self._send_request(method, url, body, headers, encode_chunked)
  File "/usr/lib/python3.7/http/client.py", line 1306, in _send_request
    self.endheaders(body, encode_chunked=encode_chunked)
  File "/usr/lib/python3.7/http/client.py", line 1255, in endheaders
    self._send_output(message_body, encode_chunked=encode_chunked)
  File "/usr/lib/python3.7/http/client.py", line 1030, in _send_output
    self.send(msg)
  File "/usr/lib/python3.7/http/client.py", line 970, in send
    self.connect()
  File "/usr/lib/python3.7/http/client.py", line 942, in connect
    (self.host,self.port), self.timeout, self.source_address)
  File "/usr/lib/python3.7/socket.py", line 727, in create_connection
    raise err
  File "/usr/lib/python3.7/socket.py", line 716, in create_connection
    sock.connect(sa)
ConnectionRefusedError: [Errno 111] Connection refused

Each failure log is accessible here:
Bitcoin ABC functional tests: abc_p2p_proof_inventory.py

Fabien planned changes to this revision.May 12 2021, 14:18
Fabien added inline comments.
src/init.cpp
1240 ↗(On Diff #28418)

I intended to remove this flag once the feature is complete, not extending it. The only use is to avoid exposing the node to network requests with no protection.

src/net_processing.cpp
3177 ↗(On Diff #28422)

Good catch, I moved the block without the continue statement. I have no preference on this or a switch, if you do let me know and I'll update

Fabien abandoned this revision.Jun 4 2021, 19:56

superseded by D9582

Revision Contents
Changeset List

PathSize
src/
avalanche/
7 lines
6 lines
8 lines
8 lines
4 lines
71 lines
test/
functional/
57 lines

Diff 28415

View Options

src/avalanche/peermanager.h

Loading...
View Options

src/avalanche/peermanager.cpp

Loading...
View Options

src/avalanche/processor.h

Loading...
View Options

src/avalanche/processor.cpp

Loading...
View Options

src/init.cpp

Loading...
View Options

src/net_processing.cpp

Loading...
View Options

test/functional/abc_p2p_proof_inventory.py

Loading...