Skip to content

Commit bbab2e3

Browse files
wqxoxowqxoxo
committed
setconfig: fix crash on dynamic multi-value plugin options
We had an assert(!(ot->type & OPT_MULTI)) which crashed when using setconfig on a plugin option marked as both dynamic and multi. The fix changes plugin_set_dynamic_opt to accept an array of values (scalar options pass a 1-element array, multi options pass the complete set). For multi options, setconfig replaces ALL values atomically - an empty array clears them. Fixes: #8295 Changelog-Fixed: setconfig no longer crashes on dynamic multi-value plugin options
1 parent 22f7a62 commit bbab2e3

14 files changed

Lines changed: 952 additions & 563 deletions

File tree

‎.msggen.json‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3706,10 +3706,12 @@
37063706
"SetConfig.config.plugin": 3,
37073707
"SetConfig.config.set": 5,
37083708
"SetConfig.config.source": 2,
3709+
"SetConfig.config.sources[]": 10,
37093710
"SetConfig.config.value_bool": 9,
37103711
"SetConfig.config.value_int": 8,
37113712
"SetConfig.config.value_msat": 7,
3712-
"SetConfig.config.value_str": 6
3713+
"SetConfig.config.value_str": 6,
3714+
"SetConfig.config.values_str[]": 11
37133715
},
37143716
"SetconfigRequest": {
37153717
"SetConfig.config": 1,
@@ -13028,6 +13030,10 @@
1302813030
"added": "pre-v0.10.1",
1302913031
"deprecated": null
1303013032
},
13033+
"SetConfig.config.sources[]": {
13034+
"added": "v26.03",
13035+
"deprecated": null
13036+
},
1303113037
"SetConfig.config.value_bool": {
1303213038
"added": "pre-v0.10.1",
1303313039
"deprecated": null
@@ -13044,6 +13050,10 @@
1304413050
"added": "pre-v0.10.1",
1304513051
"deprecated": null
1304613052
},
13053+
"SetConfig.config.values_str[]": {
13054+
"added": "v26.03",
13055+
"deprecated": null
13056+
},
1304713057
"SetConfig.transient": {
1304813058
"added": "v25.02",
1304913059
"deprecated": null

‎cln-grpc/proto/node.proto‎

Lines changed: 3 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎cln-grpc/src/convert.rs‎

Lines changed: 5 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎cln-rpc/src/model.rs‎

Lines changed: 6 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎common/configvar.c‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,9 +106,18 @@ void configvar_finalize_overrides(struct configvar **cvs)
106106
opts = tal_arr(tmpctx, const struct opt_table *, tal_count(cvs));
107107
for (size_t i = 0; i < tal_count(cvs); i++) {
108108
opts[i] = opt_find_long(cvs[i]->optvar, NULL);
109-
/* If you're allowed multiple, they don't override */
110-
if (opts[i]->type & OPT_MULTI)
109+
/* If you're allowed multiple, they don't override...
110+
* unless transient values exist, which override non-transient. */
111+
if (opts[i]->type & OPT_MULTI) {
112+
if (cvs[i]->src != CONFIGVAR_SETCONFIG_TRANSIENT)
113+
continue;
114+
for (size_t j = 0; j < i; j++) {
115+
if (opts[j] == opts[i] &&
116+
cvs[j]->src != CONFIGVAR_SETCONFIG_TRANSIENT)
117+
cvs[j]->overridden = true;
118+
}
111119
continue;
120+
}
112121
for (size_t j = 0; j < i; j++) {
113122
if (opts[j] == opts[i])
114123
cvs[j]->overridden = true;

‎contrib/msggen/msggen/schema.json‎

Lines changed: 62 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32674,10 +32674,19 @@
3267432674
},
3267532675
{
3267632676
"type": "boolean"
32677+
},
32678+
{
32679+
"type": "array",
32680+
"items": {
32681+
"type": "string"
32682+
},
32683+
"description": [
32684+
"For multi options, an array of string values."
32685+
]
3267732686
}
3267832687
],
3267932688
"description": [
32680-
"Value of the config variable to be set or updated."
32689+
"Value of the config variable to be set or updated. For multi options, this must be an array of strings."
3268132690
]
3268232691
},
3268332692
"transient": {
@@ -32704,7 +32713,6 @@
3270432713
"additionalProperties": false,
3270532714
"required": [
3270632715
"config",
32707-
"source",
3270832716
"dynamic"
3270932717
],
3271032718
"properties": {
@@ -32717,7 +32725,17 @@
3271732725
"source": {
3271832726
"type": "string",
3271932727
"description": [
32720-
"Source of configuration setting (`file`:`linenum`)."
32728+
"Source of configuration setting (`file`:`linenum`) for non-multi options."
32729+
]
32730+
},
32731+
"sources": {
32732+
"type": "array",
32733+
"added": "v26.03",
32734+
"items": {
32735+
"type": "string"
32736+
},
32737+
"description": [
32738+
"Sources of configuration settings (`file`:`linenum`) for multi options."
3272132739
]
3272232740
},
3272332741
"plugin": {
@@ -32764,6 +32782,16 @@
3276432782
"description": [
3276532783
"For boolean options."
3276632784
]
32785+
},
32786+
"values_str": {
32787+
"type": "array",
32788+
"added": "v26.03",
32789+
"items": {
32790+
"type": "string"
32791+
},
32792+
"description": [
32793+
"For multi-string options."
32794+
]
3276732795
}
3276832796
}
3276932797
}
@@ -32821,6 +32849,37 @@
3282132849
"dynamic": true
3282232850
}
3282332851
}
32852+
},
32853+
{
32854+
"description": [
32855+
"This shows setting a multi-value dynamic plugin option (requires a plugin that defines such an option)."
32856+
],
32857+
"request": {
32858+
"id": "example:setconfig#3",
32859+
"method": "setconfig",
32860+
"params": {
32861+
"config": "my-multi-option",
32862+
"val": [
32863+
"value1",
32864+
"value2"
32865+
]
32866+
}
32867+
},
32868+
"response": {
32869+
"config": {
32870+
"config": "my-multi-option",
32871+
"values_str": [
32872+
"value1",
32873+
"value2"
32874+
],
32875+
"sources": [
32876+
"/tmp/.lightning/regtest/config.setconfig:4",
32877+
"/tmp/.lightning/regtest/config.setconfig:5"
32878+
],
32879+
"plugin": "/root/lightning/plugins/myplugin",
32880+
"dynamic": true
32881+
}
32882+
}
3282432883
}
3282532884
]
3282632885
},

‎contrib/pyln-grpc-proto/pyln/grpc/node_pb2.py‎

Lines changed: 520 additions & 520 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎contrib/pyln-testing/pyln/testing/grpc2py.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2021,6 +2021,8 @@ def setchannel2py(m):
20212021

20222022
def setconfig_config2py(m):
20232023
return remove_default({
2024+
"sources": [m.sources for i in m.sources], # ArrayField[primitive] in generate_composite
2025+
"values_str": [m.values_str for i in m.values_str], # ArrayField[primitive] in generate_composite
20242026
"config": m.config, # PrimitiveField in generate_composite
20252027
"dynamic": m.dynamic, # PrimitiveField in generate_composite
20262028
"plugin": m.plugin, # PrimitiveField in generate_composite

‎doc/schemas/setconfig.json‎

Lines changed: 62 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,19 @@
3333
},
3434
{
3535
"type": "boolean"
36+
},
37+
{
38+
"type": "array",
39+
"items": {
40+
"type": "string"
41+
},
42+
"description": [
43+
"For multi options, an array of string values."
44+
]
3645
}
3746
],
3847
"description": [
39-
"Value of the config variable to be set or updated."
48+
"Value of the config variable to be set or updated. For multi options, this must be an array of strings."
4049
]
4150
},
4251
"transient": {
@@ -63,7 +72,6 @@
6372
"additionalProperties": false,
6473
"required": [
6574
"config",
66-
"source",
6775
"dynamic"
6876
],
6977
"properties": {
@@ -76,7 +84,17 @@
7684
"source": {
7785
"type": "string",
7886
"description": [
79-
"Source of configuration setting (`file`:`linenum`)."
87+
"Source of configuration setting (`file`:`linenum`) for non-multi options."
88+
]
89+
},
90+
"sources": {
91+
"type": "array",
92+
"added": "v26.03",
93+
"items": {
94+
"type": "string"
95+
},
96+
"description": [
97+
"Sources of configuration settings (`file`:`linenum`) for multi options."
8098
]
8199
},
82100
"plugin": {
@@ -123,6 +141,16 @@
123141
"description": [
124142
"For boolean options."
125143
]
144+
},
145+
"values_str": {
146+
"type": "array",
147+
"added": "v26.03",
148+
"items": {
149+
"type": "string"
150+
},
151+
"description": [
152+
"For multi-string options."
153+
]
126154
}
127155
}
128156
}
@@ -180,6 +208,37 @@
180208
"dynamic": true
181209
}
182210
}
211+
},
212+
{
213+
"description": [
214+
"This shows setting a multi-value dynamic plugin option (requires a plugin that defines such an option)."
215+
],
216+
"request": {
217+
"id": "example:setconfig#3",
218+
"method": "setconfig",
219+
"params": {
220+
"config": "my-multi-option",
221+
"val": [
222+
"value1",
223+
"value2"
224+
]
225+
}
226+
},
227+
"response": {
228+
"config": {
229+
"config": "my-multi-option",
230+
"values_str": [
231+
"value1",
232+
"value2"
233+
],
234+
"sources": [
235+
"/tmp/.lightning/regtest/config.setconfig:4",
236+
"/tmp/.lightning/regtest/config.setconfig:5"
237+
],
238+
"plugin": "/root/lightning/plugins/myplugin",
239+
"dynamic": true
240+
}
241+
}
183242
}
184243
]
185244
}

0 commit comments

Comments
 (0)