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

feat(comms)!: add signature to peer identity to allow third party identity updates #3629

Merged
merged 29 commits into from Jan 7, 2022
Merged

feat(comms)!: add signature to peer identity to allow third party identity updates #3629

merged 29 commits into from Jan 7, 2022

Conversation

sdbondi
Copy link
Member

@sdbondi sdbondi commented Nov 30, 2021 •
edited

Description

  • add signature to peers with update timestamp to allow updates
  • update peer sync, identity, discovery and join protocols to validate signature (backward compatible)
  • signature is optional, if not provided the peer addresses is not updated
  • add migration for peer db
  • remove old migrations
  • add some unit tests
  • include PR fix: use json 5 for tor identity (regression) #3624
  • fix: use of from_timestamp on unvalidated data can cause a panic, replaced with from_timestamp_opt
  • decouple comms cipher key from wallet and implement identity signing after loading (👁️ 🙏 @philipr-za )
  • add list connected public keys to FFI (👁️ 🙏 @StriderDM)
  • update cucumber tests as needed

Compatibility
PeerDB: All changes are forward compatible, once upgraded the previous peer db cannot be used.
DhtProtocol: fully compatible, identity signature is optional and will have the same effect as the current protocol if not provided
Identity Protocol: fully compatible, identity signature is optional but SHOULD be provided once upgraded
NodeIdentity json file: will be updated on node startup

Motivation and Context

Allow address changes to be securely propagated through the network by third party peers

How Has This Been Tested?

Some basic unit tests
Compatibility: manually with two upgraded base nodes connecting to existing weatherwax nodes, console wallet tested

@sdbondi sdbondi changed the title feat!(dht): add signature to peer identity to allow updates feat(dht)!: add signature to peer identity to allow updates Nov 30, 2021
@sdbondi sdbondi force-pushed the comms-peer-manager-signed-peers branch 4 times, most recently from d371aed to 311cffe Compare November 30, 2021 07:12
delta1
delta1 reviewed Nov 30, 2021
View reviewed changes
Copy link
Contributor

@delta1 delta1 left a comment

Choose a reason for hiding this comment

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

Nice looking really good, seems to be a build issue that's blocking CI

applications/tari_app_utilities/src/identity_management.rs Outdated Show resolved Hide resolved
applications/tari_app_utilities/src/identity_management.rs Outdated Show resolved Hide resolved
@sdbondi sdbondi force-pushed the comms-peer-manager-signed-peers branch from 311cffe to 909460c Compare November 30, 2021 07:17
@sdbondi sdbondi changed the title feat(dht)!: add signature to peer identity to allow updates feat(comms)!: add signature to peer identity to allow updates Nov 30, 2021
@sdbondi sdbondi force-pushed the comms-peer-manager-signed-peers branch from d4a9d0e to 21e7d37 Compare November 30, 2021 12:42
@sdbondi sdbondi changed the title feat(comms)!: add signature to peer identity to allow updates feat(comms)!: add signature to peer identity to allow third party identity updates Nov 30, 2021
@delta1
Copy link
Contributor

delta1 commented Dec 6, 2021

Needs conflict resolution

@sdbondi
Merge branch 'development' into comms-peer-manager-signed-peers
0b11c19 
* development:
  feat(consensus)!: add tari script byte size limit check to validation (tari-project#3640)
  fix: minor improvements to available neighbouring peer search (tari-project#3598)
  feat: bad block list for invalid blocks after sync (tari-project#3637)
@sdbondi
v5 migration
ed04dea
@sdbondi sdbondi force-pushed the comms-peer-manager-signed-peers branch from 11ab088 to ed04dea Compare December 7, 2021 05:27
@sdbondi
Copy link
Member Author

sdbondi commented Dec 7, 2021

@delta1 Done

@sdbondi sdbondi mentioned this pull request Dec 7, 2021
Closed
@delta1
Copy link
Contributor

delta1 commented Dec 7, 2021

That FFI test seems to be failing reliably. Going to try it out locally but I think there's may be a breaking change there?

@delta1
Copy link
Contributor

delta1 commented Dec 7, 2021

Or is it just a flaky test?

@sdbondi
Copy link
Member Author

sdbondi commented Dec 7, 2021 •
edited

@delta1 Seems worth looking into since this touches FFI - will take a look

EDIT: Will give it a second run, since it's a bit heavy to run on my machine right now

@delta1
Copy link
Contributor

delta1 commented Dec 7, 2021

Same issue reproduced locally @sdbondi

@sdbondi sdbondi marked this pull request as draft December 8, 2021 04:48
@sdbondi sdbondi marked this pull request as ready for review December 8, 2021 11:39
@sdbondi
Merge branch 'development' into comms-peer-manager-signed-peers
481cade 
* development:
  chore: add eslint to explorer (tari-project#3648)
  v0.22.0
  perf: set MDB_NOLOCK for peer and blockchain database (tari-project#3473)
  feat: prevent banning of connected base node in wallet (tari-project#3642)
@sdbondi sdbondi force-pushed the comms-peer-manager-signed-peers branch from 4241ddd to 481cade Compare December 8, 2021 11:43
@sdbondi
Copy link
Member Author

sdbondi commented Dec 8, 2021 •
edited

@delta1 Ended up being an unrelated outbound messaging issue where the base node holds onto the outbound messaging stream for longer than it should after the peer disconnects (FFI_WALLET in this case), sending the SAF messages on a stale stream so it never arrives even though it was successfully requested by the new connection after the restart. Added a change that immediately terminates the outbound stream on peer disconnect which seems to have fixed it (and improved the overall reliability of the messaging protocol). Not clear why this becomes more prevalent in this PR. Thanks for picking up the failed FFI test.

@sdbondi
missing word in cucumber test
7fd71d4
@sdbondi
Merge branch 'development' into comms-peer-manager-signed-peers
3abefcd 
* development: (77 commits)
  chore: merge development
  refactor: split transaction file (tari-project#3653)
  refactor: change transaction mod name
  chore: fix imports fmt (tari-project#3650)
  feat: add 721 template stub (tari-project#3643)
  feat: add transfer method to tip002 (tari-project#3632)
  feat: add connect per site into the web extension (tari-project#3626)
  refactor: fix db structure (tari-project#3616)
  feat: add asset key manager to collectibles (tari-project#3609)
  ci: update validator-node CI for faster feedback (tari-project#3606)
  feat: add state database (tari-project#3591)
  feat: add wallet stub to collectibles app (tari-project#3589)
  feat: add onboarding flow for web extension (tari-project#3582)
  feat: compile key manager to wasm and add javascript interface (tari-project#3565)
  feat: add storage to validator node (tari-project#3551)
  fix: wallet was not able to run (tari-project#3561)
  feat: add react popup with some basic functionality (tari-project#3550)
  feat: create tari web extension application stub (tari-project#3535)
  chore: merge dev
  refactor: move conversion
  ...
@sdbondi
Merge branch 'development' into comms-peer-manager-signed-peers
7136f66
@sdbondi
Merge branch 'development' into comms-peer-manager-signed-peers
f0675f1 
* development:
  fix: allow 0-conf in blockchain db (tari-project#3680)
  test: fix cucumber WalletQuery.feature (tari-project#3677)
  test: fix `wait for` step (tari-project#3673)
  perf(comms)!: optimise connection establishment (tari-project#3658)
  fix: remove noise negotiation for debugging on bad wire mode (tari-project#3657)
  feat: add follow on checkpoint (tari-project#3676)
  ci: fix tari collectibles build (tari-project#3670)
  feat: example to generate vanity node_ids (tari-project#3654)
  fix: prefer configured seeds over dns seeds (tari-project#3662)
  docs: update RFC-0230_HTLC to use TariScript (tari-project#3622)
  feat: add invoke write method to proxy (tari-project#3667)
  ci: add tauri build (tari-project#3669)
  test: fix cucumber metadata signature (tari-project#3665)
  feat: add asset proxy (tari-project#3659)
  test: fix cucumber (tari-project#3660)
@sdbondi sdbondi force-pushed the comms-peer-manager-signed-peers branch from 1814d89 to f0675f1 Compare January 5, 2022 09:03
@sdbondi
Merge branch 'development' into comms-peer-manager-signed-peers
352e95c 
* development:
  chore: remove moving lock.mdb (tari-project#3674)
  chore: merge weatherwax
  feat!: provide a compact form of TransactionInput (tari-project#3460)
  v0.22.1.1
  v0.22.1
  ci: add build step (tari-project#3678)
  fix: edge cases causing bans during header/block sync (tari-project#3661)
  fix: end stale outbound queue immediately on disconnect, retry outbound messages (tari-project#3664)
  feat: add search by commitment to explorer (tari-project#3668)
  feat: tari launchpad (tari-project#3671)
  feat: base_node switching for console_wallet when status is offline (tari-project#3639)
  feat: improve wallet recovery and scanning handling of reorgs (tari-project#3655)
  feat: add GRPC call to search for utxo via commitment hex (tari-project#3666)
  feat: custom_base_node in config (tari-project#3651)
  fix: return correct index for include_pruned_utxos = false (tari-project#3663)
@stringhandler stringhandler merged commit c672d48 into tari-project:development Jan 7, 2022
@sdbondi sdbondi deleted the comms-peer-manager-signed-peers branch January 7, 2022 11:50
sdbondi added a commit to sdbondi/tari that referenced this pull request Jan 10, 2022
@sdbondi
Merge branch 'development' into core-covenants-initial-impl
94c2bfc 
* development:
  feat: dibbler new genesis block with faucet utxos (tari-project#3688)
  ci: fix clippy warning on generated proto module (tari-project#3690)
  test: fix metadata signature cucumber (tari-project#3687)
  refactor!: clean up #testnet reset TODOs (tari-project#3682)
  feat(comms)!: add signature to peer identity to allow third party identity updates (tari-project#3629)
  chore: remove moving lock.mdb (tari-project#3674)
  chore: merge weatherwax
  v0.22.1.1
  v0.22.1
  ci: add build step (tari-project#3678)
  fix: edge cases causing bans during header/block sync (tari-project#3661)
  fix: end stale outbound queue immediately on disconnect, retry outbound messages (tari-project#3664)
  feat: add search by commitment to explorer (tari-project#3668)
  feat: tari launchpad (tari-project#3671)
  feat: base_node switching for console_wallet when status is offline (tari-project#3639)
  feat: improve wallet recovery and scanning handling of reorgs (tari-project#3655)
  feat: add GRPC call to search for utxo via commitment hex (tari-project#3666)
  feat: custom_base_node in config (tari-project#3651)
  fix: return correct index for include_pruned_utxos = false (tari-project#3663)
sdbondi added a commit to sdbondi/tari that referenced this pull request Jan 11, 2022
@sdbondi
Merge branch 'development' into core-covenants-integration
d57329f 
* development: (28 commits)
  feat: covenants implementation (tari-project#3656)
  ci: add tor script to binaries bundle (tari-project#3689)
  chore: remove testnet reset todo in wallet (tari-project#3685)
  feat: dibbler new genesis block with faucet utxos (tari-project#3688)
  ci: fix clippy warning on generated proto module (tari-project#3690)
  test: fix metadata signature cucumber (tari-project#3687)
  refactor!: clean up #testnet reset TODOs (tari-project#3682)
  feat(comms)!: add signature to peer identity to allow third party identity updates (tari-project#3629)
  chore: remove moving lock.mdb (tari-project#3674)
  chore: merge weatherwax
  feat!: provide a compact form of TransactionInput (tari-project#3460)
  fix: allow 0-conf in blockchain db (tari-project#3680)
  v0.22.1.1
  v0.22.1
  ci: add build step (tari-project#3678)
  test: fix cucumber WalletQuery.feature (tari-project#3677)
  test: fix `wait for` step (tari-project#3673)
  fix: edge cases causing bans during header/block sync (tari-project#3661)
  perf(comms)!: optimise connection establishment (tari-project#3658)
  fix: end stale outbound queue immediately on disconnect, retry outbound messages (tari-project#3664)
  ...
@sdbondi sdbondi restored the comms-peer-manager-signed-peers branch February 3, 2022 05:31
@sdbondi sdbondi deleted the comms-peer-manager-signed-peers branch November 29, 2022 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@delta1 delta1 delta1 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@sdbondi @delta1 @stringhandler