Skip to content

Commit e9fee87

Browse files
wqxoxorustyrussell
wqxoxo
authored and
rustyrussell
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 2346d37 commit e9fee87

14 files changed

Lines changed: 1086 additions & 697 deletions

File tree

‎.msggen.json‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4060,10 +4060,12 @@
40604060
"SetConfig.config.plugin": 3,
40614061
"SetConfig.config.set": 5,
40624062
"SetConfig.config.source": 2,
4063+
"SetConfig.config.sources[]": 10,
40634064
"SetConfig.config.value_bool": 9,
40644065
"SetConfig.config.value_int": 8,
40654066
"SetConfig.config.value_msat": 7,
4066-
"SetConfig.config.value_str": 6
4067+
"SetConfig.config.value_str": 6,
4068+
"SetConfig.config.values_str[]": 11
40674069
},
40684070
"SetconfigRequest": {
40694071
"SetConfig.config": 1,
@@ -13757,6 +13759,10 @@
1375713759
"added": "v23.08",
1375813760
"deprecated": null
1375913761
},
13762+
"SetConfig.config.sources[]": {
13763+
"added": "v26.06",
13764+
"deprecated": null
13765+
},
1376013766
"SetConfig.config.value_bool": {
1376113767
"added": "v23.08",
1376213768
"deprecated": null
@@ -13773,6 +13779,10 @@
1377313779
"added": "v23.08",
1377413780
"deprecated": null
1377513781
},
13782+
"SetConfig.config.values_str[]": {
13783+
"added": "v26.06",
13784+
"deprecated": null
13785+
},
1377613786
"SetConfig.transient": {
1377713787
"added": "v25.02",
1377813788
"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
@@ -107,9 +107,18 @@ void configvar_finalize_overrides(struct configvar **cvs)
107107
opts = tal_arr(tmpctx, const struct opt_table *, tal_count(cvs));
108108
for (size_t i = 0; i < tal_count(cvs); i++) {
109109
opts[i] = opt_find_long(cvs[i]->optvar, NULL);
110-
/* If you're allowed multiple, they don't override */
111-
if (opts[i]->type & OPT_MULTI)
110+
/* If you're allowed multiple, they don't override...
111+
* unless transient values exist, which override non-transient. */
112+
if (opts[i]->type & OPT_MULTI) {
113+
if (cvs[i]->src != CONFIGVAR_SETCONFIG_TRANSIENT)
114+
continue;
115+
for (size_t j = 0; j < i; j++) {
116+
if (opts[j] == opts[i] &&
117+
cvs[j]->src != CONFIGVAR_SETCONFIG_TRANSIENT)
118+
cvs[j]->overridden = true;
119+
}
112120
continue;
121+
}
113122
for (size_t j = 0; j < i; j++) {
114123
if (opts[j] == opts[i])
115124
cvs[j]->overridden = true;

‎contrib/msggen/msggen/schema.json‎

Lines changed: 62 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33890,10 +33890,19 @@
3389033890
},
3389133891
{
3389233892
"type": "boolean"
33893+
},
33894+
{
33895+
"type": "array",
33896+
"items": {
33897+
"type": "string"
33898+
},
33899+
"description": [
33900+
"For multi options, an array of string values."
33901+
]
3389333902
}
3389433903
],
3389533904
"description": [
33896-
"Value of the config variable to be set or updated."
33905+
"Value of the config variable to be set or updated. For multi options, this must be an array of strings."
3389733906
]
3389833907
},
3389933908
"transient": {
@@ -33920,7 +33929,6 @@
3392033929
"additionalProperties": false,
3392133930
"required": [
3392233931
"config",
33923-
"source",
3392433932
"dynamic"
3392533933
],
3392633934
"properties": {
@@ -33933,7 +33941,17 @@
3393333941
"source": {
3393433942
"type": "string",
3393533943
"description": [
33936-
"Source of configuration setting (`file`:`linenum`)."
33944+
"Source of configuration setting (`file`:`linenum`) for non-multi options."
33945+
]
33946+
},
33947+
"sources": {
33948+
"type": "array",
33949+
"added": "v26.06",
33950+
"items": {
33951+
"type": "string"
33952+
},
33953+
"description": [
33954+
"Sources of configuration settings (`file`:`linenum`) for multi options."
3393733955
]
3393833956
},
3393933957
"plugin": {
@@ -33980,6 +33998,16 @@
3398033998
"description": [
3398133999
"For boolean options."
3398234000
]
34001+
},
34002+
"values_str": {
34003+
"type": "array",
34004+
"added": "v26.06",
34005+
"items": {
34006+
"type": "string"
34007+
},
34008+
"description": [
34009+
"For multi-string options."
34010+
]
3398334011
}
3398434012
}
3398534013
}
@@ -34037,6 +34065,37 @@
3403734065
"dynamic": true
3403834066
}
3403934067
}
34068+
},
34069+
{
34070+
"description": [
34071+
"This shows setting a multi-value dynamic plugin option (requires a plugin that defines such an option)."
34072+
],
34073+
"request": {
34074+
"id": "example:setconfig#3",
34075+
"method": "setconfig",
34076+
"params": {
34077+
"config": "my-multi-option",
34078+
"val": [
34079+
"value1",
34080+
"value2"
34081+
]
34082+
}
34083+
},
34084+
"response": {
34085+
"config": {
34086+
"config": "my-multi-option",
34087+
"values_str": [
34088+
"value1",
34089+
"value2"
34090+
],
34091+
"sources": [
34092+
"/tmp/.lightning/regtest/config.setconfig:4",
34093+
"/tmp/.lightning/regtest/config.setconfig:5"
34094+
],
34095+
"plugin": "/root/lightning/plugins/myplugin",
34096+
"dynamic": true
34097+
}
34098+
}
3404034099
}
3404134100
]
3404234101
},

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

Lines changed: 654 additions & 654 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
@@ -2015,6 +2015,8 @@ def setchannel2py(m):
20152015

20162016
def setconfig_config2py(m):
20172017
return remove_default({
2018+
"sources": [m.sources for i in m.sources], # ArrayField[primitive] in generate_composite
2019+
"values_str": [m.values_str for i in m.values_str], # ArrayField[primitive] in generate_composite
20182020
"config": m.config, # PrimitiveField in generate_composite
20192021
"dynamic": m.dynamic, # PrimitiveField in generate_composite
20202022
"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.06",
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.06",
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)