tari-project / tari

The Tari protocol
https://tari.com
BSD 3-Clause "New" or "Revised" License
356 stars 220 forks source link

feat: validate blockchain fetch from DNS server with local state #6658

Closed MCozhusheck closed 1 week ago

MCozhusheck commented 4 weeks ago

Description

Add new service which will periodically get checkpoints from a DNS server and validate with it's own state. If we fail this check we will include this information in get_new_block_template request so the sender is aware that this node is out of sync.

Motivation and Context

This will allow miners including one from Tari Universe to avoid mining on the orphan chain. This implementation is modeled after the Monero Pulse

How Has This Been Tested?

Run app on the nextnet and look for logs with tari_pulse in the base_layer.log file like this: tail -f base_layer.log | grep tari_pulse

Make get_new_block_template and verify if new fields are present.

Create orphan chain (eg. by turning off tor and banning all TCP peers) and check (with above methods) if nodes detects orphan chain.

What process can a PR reviewer use to test or verify this change?

Same as above

Breaking Changes

MCozhusheck commented 1 week ago

Support for other chains than nextnet have yet to be implemented after these dns servers are up and running.

github-actions[bot] commented 1 week ago

Test Results (CI)

    3 files    129 suites   40m 44s ⏱️ 1 344 tests 1 344 ✅ 0 💤 0 ❌ 4 030 runs  4 030 ✅ 0 💤 0 ❌

Results for commit 877ec626.

:recycle: This comment has been updated with latest results.

github-actions[bot] commented 1 week ago

Test Results (Integration tests)

 2 files  + 2  11 suites  +11   16m 58s ⏱️ + 16m 58s 36 tests +36  35 ✅ +35  0 💤 ±0  1 ❌ +1  37 runs  +37  36 ✅ +36  0 💤 ±0  1 ❌ +1 

For more details on these failures, see this check.

Results for commit 877ec626. ± Comparison against base commit 10f4077a.

:recycle: This comment has been updated with latest results.

hansieodendaal commented 1 week ago

I am unsure about this approach altogether, as it centralizes authority and appoints the DNS server as an oracle that can influence outcomes.

MCozhusheck commented 1 week ago

I am unsure about this approach altogether, as it centralizes authority and appoints the DNS server as an oracle that can influence outcomes.

This is only used to hint that this node might be not connected to the rest of the network. This was common issue in Tari Universe where most of the network was connected through Tor but after disabling this feature in the settings, users where trying to connect through TCP where there no other nodes. There are some other concerns that required Monero to introduce Monero Pulse which solved 1 vulnerability regarding invalid historical block in the chain.