Skip to content

Commit b53a45e

Browse files
Address review: reject commas in wallet addrs, dedupe addr codec
- Validate() rejects a comma in the primary or any fallback wallet address, since they're persisted comma-separated (a comma would corrupt the column). - Move the comma-separated encode/decode into domain (EncodeFallbackAddrs / DecodeFallbackAddrs) so postgres and sqlite share one implementation. - Document the wallet-target trust boundary in walletService and log the effective primary wallet address at startup so operators can audit it.
1 parent 49f4be4 commit b53a45e

7 files changed

Lines changed: 62 additions & 44 deletions

File tree

internal/config/config.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -781,6 +781,10 @@ func (c *Config) walletService() error {
781781
// The wallet addresses are sourced from the persisted settings (seeded from
782782
// env on first boot), so admin changes take effect on the next restart. Fall
783783
// back to env if settings aren't available yet (e.g. the repo isn't wired).
784+
//
785+
// Trust boundary: the wallet gRPC target controls signing, address derivation
786+
// and sweeps, so persisting it via the admin Settings API is equivalent to full
787+
// operator (admin macaroon) trust — the same trust needed to change the env var.
784788
arkWallet := c.WalletAddr
785789
fallbackAddrs := c.WalletFallbackAddrs
786790
if c.repo != nil {
@@ -813,6 +817,10 @@ func (c *Config) walletService() error {
813817
c.wallet = walletSvc
814818
c.network = network
815819

820+
// Surface the effective wallet target so operators can audit which arkd-wallet
821+
// this node dialed (it may come from the DB, not the env var).
822+
log.Infof("dialed primary arkd-wallet at %q on network %s", arkWallet, network.Name)
823+
816824
fallbacks, err := c.dialFallbackWallets(fallbackAddrs)
817825
if err != nil {
818826
return err

internal/core/domain/settings.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package domain
22

33
import (
44
"fmt"
5+
"strings"
56
"time"
67

78
arklib "github.com/arkade-os/arkd/pkg/ark-lib"
@@ -235,9 +236,41 @@ func (s Settings) Validate() error {
235236
s.RoundMaxParticipantsCount, s.RoundMinParticipantsCount,
236237
)
237238
}
239+
240+
// Fallback addrs are persisted comma-separated by the SQL backends, so a comma
241+
// in an address would corrupt the column on the next read.
242+
if strings.Contains(s.WalletAddr, ",") {
243+
return fmt.Errorf("wallet addr must not contain a comma")
244+
}
245+
for _, addr := range s.WalletFallbackAddrs {
246+
if strings.Contains(addr, ",") {
247+
return fmt.Errorf("wallet fallback addr %q must not contain a comma", addr)
248+
}
249+
}
238250
return nil
239251
}
240252

253+
// EncodeFallbackAddrs and DecodeFallbackAddrs convert WalletFallbackAddrs to and
254+
// from the comma-separated form the SQL backends persist it in. Decoding trims
255+
// whitespace and drops empty entries; an empty result is returned as nil.
256+
func EncodeFallbackAddrs(addrs []string) string {
257+
return strings.Join(addrs, ",")
258+
}
259+
260+
func DecodeFallbackAddrs(s string) []string {
261+
parts := strings.Split(s, ",")
262+
addrs := make([]string, 0, len(parts))
263+
for _, p := range parts {
264+
if p = strings.TrimSpace(p); p != "" {
265+
addrs = append(addrs, p)
266+
}
267+
}
268+
if len(addrs) == 0 {
269+
return nil
270+
}
271+
return addrs
272+
}
273+
241274
// SettingsUpdate is a copy of the Settings repo struct, but with optional fields to easily handle
242275
// changes.
243276
type SettingsUpdate struct {

internal/core/domain/settings_test.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,12 @@ func testValidateSettings(t *testing.T) {
138138
zeroMaxOpReturn := validSettings
139139
zeroMaxOpReturn.MaxOpReturnOutputs = 0
140140

141+
commaWalletAddr := validSettings
142+
commaWalletAddr.WalletAddr = "host:6060,evil"
143+
144+
commaFallbackAddr := validSettings
145+
commaFallbackAddr.WalletFallbackAddrs = []string{"host:6060,evil"}
146+
141147
fixtures := []struct {
142148
settings domain.Settings
143149
expectedErr string
@@ -239,6 +245,15 @@ func testValidateSettings(t *testing.T) {
239245
settings: zeroMaxOpReturn,
240246
expectedErr: "max op return outputs must be greater than 0",
241247
},
248+
{
249+
settings: commaWalletAddr,
250+
expectedErr: "wallet addr must not contain a comma",
251+
},
252+
{
253+
settings: commaFallbackAddr,
254+
expectedErr: `wallet fallback addr "host:6060,evil" ` +
255+
"must not contain a comma",
256+
},
242257
}
243258

244259
for _, f := range fixtures {

internal/infrastructure/db/postgres/settings_repo.go

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"database/sql"
66
"errors"
77
"fmt"
8-
"strings"
98
"sync"
109
"time"
1110

@@ -14,23 +13,6 @@ import (
1413
arklib "github.com/arkade-os/arkd/pkg/ark-lib"
1514
)
1615

17-
// splitFallbackAddrs decodes the comma-separated wallet_fallback_addrs column
18-
// back into a slice, trimming whitespace and dropping empty entries (so it
19-
// normalizes the same way the env parser does); an empty result is returned as nil.
20-
func splitFallbackAddrs(s string) []string {
21-
parts := strings.Split(s, ",")
22-
addrs := make([]string, 0, len(parts))
23-
for _, p := range parts {
24-
if p = strings.TrimSpace(p); p != "" {
25-
addrs = append(addrs, p)
26-
}
27-
}
28-
if len(addrs) == 0 {
29-
return nil
30-
}
31-
return addrs
32-
}
33-
3416
type settingsRepository struct {
3517
db *sql.DB
3618
querier *queries.Queries
@@ -114,7 +96,7 @@ func (r *settingsRepository) Get(ctx context.Context) (*domain.Settings, error)
11496
AssetTxMaxWeightRatio: row.AssetTxMaxWeightRatio,
11597
NoteUriPrefix: row.NoteUriPrefix,
11698
WalletAddr: row.WalletAddr,
117-
WalletFallbackAddrs: splitFallbackAddrs(row.WalletFallbackAddrs),
99+
WalletFallbackAddrs: domain.DecodeFallbackAddrs(row.WalletFallbackAddrs),
118100
ScheduledSession: scheduledSession,
119101
BatchFees: domain.BatchFees{
120102
OnchainInputFee: row.BatchOnchainInputFee,
@@ -155,7 +137,7 @@ func (r *settingsRepository) Upsert(
155137
AssetTxMaxWeightRatio: settings.AssetTxMaxWeightRatio,
156138
NoteUriPrefix: settings.NoteUriPrefix,
157139
WalletAddr: settings.WalletAddr,
158-
WalletFallbackAddrs: strings.Join(settings.WalletFallbackAddrs, ","),
140+
WalletFallbackAddrs: domain.EncodeFallbackAddrs(settings.WalletFallbackAddrs),
159141
BatchOnchainInputFee: settings.BatchFees.OnchainInputFee,
160142
BatchOffchainInputFee: settings.BatchFees.OffchainInputFee,
161143
BatchOnchainOutputFee: settings.BatchFees.OnchainOutputFee,

internal/infrastructure/db/postgres/settings_seed.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"database/sql"
66
"errors"
77
"fmt"
8-
"strings"
98
"time"
109

1110
"github.com/arkade-os/arkd/internal/core/domain"
@@ -143,7 +142,7 @@ func seedParams(settings domain.Settings) queries.UpsertSettingsParams {
143142
AssetTxMaxWeightRatio: settings.AssetTxMaxWeightRatio,
144143
NoteUriPrefix: settings.NoteUriPrefix,
145144
WalletAddr: settings.WalletAddr,
146-
WalletFallbackAddrs: strings.Join(settings.WalletFallbackAddrs, ","),
145+
WalletFallbackAddrs: domain.EncodeFallbackAddrs(settings.WalletFallbackAddrs),
147146
BatchOnchainInputFee: settings.BatchFees.OnchainInputFee,
148147
BatchOffchainInputFee: settings.BatchFees.OffchainInputFee,
149148
BatchOnchainOutputFee: settings.BatchFees.OnchainOutputFee,

internal/infrastructure/db/sqlite/settings_repo.go

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"database/sql"
66
"errors"
77
"fmt"
8-
"strings"
98
"sync"
109
"time"
1110

@@ -14,23 +13,6 @@ import (
1413
arklib "github.com/arkade-os/arkd/pkg/ark-lib"
1514
)
1615

17-
// splitFallbackAddrs decodes the comma-separated wallet_fallback_addrs column
18-
// back into a slice, trimming whitespace and dropping empty entries (so it
19-
// normalizes the same way the env parser does); an empty result is returned as nil.
20-
func splitFallbackAddrs(s string) []string {
21-
parts := strings.Split(s, ",")
22-
addrs := make([]string, 0, len(parts))
23-
for _, p := range parts {
24-
if p = strings.TrimSpace(p); p != "" {
25-
addrs = append(addrs, p)
26-
}
27-
}
28-
if len(addrs) == 0 {
29-
return nil
30-
}
31-
return addrs
32-
}
33-
3416
type settingsRepository struct {
3517
db *sql.DB
3618
querier *queries.Queries
@@ -114,7 +96,7 @@ func (r *settingsRepository) Get(ctx context.Context) (*domain.Settings, error)
11496
AssetTxMaxWeightRatio: float32(row.AssetTxMaxWeightRatio),
11597
NoteUriPrefix: row.NoteUriPrefix,
11698
WalletAddr: row.WalletAddr,
117-
WalletFallbackAddrs: splitFallbackAddrs(row.WalletFallbackAddrs),
99+
WalletFallbackAddrs: domain.DecodeFallbackAddrs(row.WalletFallbackAddrs),
118100
ScheduledSession: scheduledSession,
119101
BatchFees: domain.BatchFees{
120102
OnchainInputFee: row.BatchOnchainInputFee,
@@ -154,7 +136,7 @@ func (r *settingsRepository) Upsert(
154136
AssetTxMaxWeightRatio: float64(settings.AssetTxMaxWeightRatio),
155137
NoteUriPrefix: settings.NoteUriPrefix,
156138
WalletAddr: settings.WalletAddr,
157-
WalletFallbackAddrs: strings.Join(settings.WalletFallbackAddrs, ","),
139+
WalletFallbackAddrs: domain.EncodeFallbackAddrs(settings.WalletFallbackAddrs),
158140
BatchOnchainInputFee: settings.BatchFees.OnchainInputFee,
159141
BatchOffchainInputFee: settings.BatchFees.OffchainInputFee,
160142
BatchOnchainOutputFee: settings.BatchFees.OnchainOutputFee,

internal/infrastructure/db/sqlite/settings_seed.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"database/sql"
66
"errors"
77
"fmt"
8-
"strings"
98
"time"
109

1110
"github.com/arkade-os/arkd/internal/core/domain"
@@ -143,7 +142,7 @@ func seedParams(settings domain.Settings) queries.UpsertSettingsParams {
143142
AssetTxMaxWeightRatio: float64(settings.AssetTxMaxWeightRatio),
144143
NoteUriPrefix: settings.NoteUriPrefix,
145144
WalletAddr: settings.WalletAddr,
146-
WalletFallbackAddrs: strings.Join(settings.WalletFallbackAddrs, ","),
145+
WalletFallbackAddrs: domain.EncodeFallbackAddrs(settings.WalletFallbackAddrs),
147146
BatchOnchainInputFee: settings.BatchFees.OnchainInputFee,
148147
BatchOffchainInputFee: settings.BatchFees.OffchainInputFee,
149148
BatchOnchainOutputFee: settings.BatchFees.OnchainOutputFee,

0 commit comments

Comments
 (0)