status-im / status-go

The Status module that consumes go-ethereum
https://status.im
Mozilla Public License 2.0
720 stars 242 forks source link

Remove `nodeConfig` argument from local pairing `ReceiverConfig` #5338

Open igor-sirotin opened 3 weeks ago

igor-sirotin commented 3 weeks ago

Problem

NodeConfig should not be directly managed by clients.

In desktop we still have to keep building default node config just for local pairing now: https://github.com/status-im/status-desktop/blob/6fadda1c7a5284d3e602e5f14e38b501cc92cd6d/src/app_service/service/devices/service.nim#L224-L237

Implementation

status-go should use defaultNodeConfig(...) with some parameters from client: https://github.com/status-im/status-go/blob/54319cf8648b6aa8c691f2c9d6216198dfe28d75/api/defaults.go#L221

Acceptance Criteria

NodeConfig is not passed by clients directly

igor-sirotin commented 3 weeks ago

@Samyoul, do you have any complaints about this?

Samyoul commented 1 week ago

@igor-sirotin I don't have a complaint about this, we only use 1 or 2 parameters during local pairing. It made sense to pass in when the app client had the config in memory but I don't have a problem at all with changing this.