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

fix: use json 5 for tor identity (regression) #3624

Merged

Conversation

sdbondi
Copy link
Member

@sdbondi sdbondi commented Nov 28, 2021

Description

Use json 5 for save/load identity functions

Motivation and Context

Save and load identity were using older version of the json parser which coul not parse the json comments.
This causes a new tor identity and onion address to be created on each base node startup.

How Has This Been Tested?

Onion address does not change on base node restart

Copy link
Contributor

@hansieodendaal hansieodendaal left a comment

Choose a reason for hiding this comment

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

LGTM

@aviator-app aviator-app bot added the mq-failed label Dec 2, 2021
@delta1 delta1 removed the mq-failed label Dec 2, 2021
@aviator-app aviator-app bot added mq-failed and removed mq-failed labels Dec 2, 2021
@aviator-app aviator-app bot merged commit 7d49fa4 into tari-project:development Dec 2, 2021
sdbondi added a commit to sdbondi/tari that referenced this pull request Dec 2, 2021
* development:
  fix: use json 5 for tor identity (regression) (tari-project#3624)
@sdbondi sdbondi deleted the base-node-json5-for-tor-id-load branch December 2, 2021 11:51
sdbondi added a commit to sdbondi/tari that referenced this pull request Dec 6, 2021
* development: (29 commits)
  fix(pruned mode)!: prune inputs, allow horizon sync resume and other fixes (tari-project#3521)
  feat!: sending one-sided transactions in wallet_ffi (tari-project#3634)
  fix: use json 5 for tor identity (regression) (tari-project#3624)
  test: add operation_id to log messages (tari-project#3633)
  fix!: multiple monerod addresses in tari merge mining proxy (tari-project#3628)
  fix: get-peer command works with public key again (tari-project#3636)
  fix!: separate peer seeds to common.network (tari-project#3635)
  test: removed stress test log target (tari-project#3631)
  feat: removed transaction validation redundant events (tari-project#3630)
  feat: improve wallet responsiveness (tari-project#3625)
  feat: add bulletproof rewind profiling (tari-project#3618)
  fix!: console wallet grpc_console_wallet_addresss config (tari-project#3619)
  test: increase timeout in cucumber (tari-project#3621)
  chore: change status line (tari-project#3610)
  feat!: add tcp bypass settings for tor in wallet_ffi (tari-project#3615)
  feat: only trigger UTXO scanning when a new block event is received (tari-project#3620)
  feat: implement dht pooled db connection (tari-project#3596)
  feat: add page for detailed mempool in explorer (tari-project#3613)
  chore: add pub key in the dailes notify (tari-project#3612)
  feat: display network for console wallet (tari-project#3611)
  ...
stringhandler pushed a commit that referenced this pull request Jan 7, 2022
…ntity updates (#3629)

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 #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 restored the base-node-json5-for-tor-id-load branch February 3, 2022 05:33
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