89@@ -90,6 +90,10 @@ def run_test(self):
90 assert_raises_rpc_error(-8, "Parameter arg1 specified twice both as positional and named argument", self.nodes[0].cli.echo, 0, 1, arg1=1)
91 assert_raises_rpc_error(-8, "Parameter arg1 specified twice both as positional and named argument", self.nodes[0].cli.echo, 0, None, 2, arg1=1)
92
93+ self.log.info("Test that later cli named arguments values silently overwrite earlier ones")
94+ assert_equal(self.nodes[0].cli("-named", "echo", "arg0=0", "arg1=1", "arg2=2", "arg1=3").send_cli(), ['0', '3', '2'])
95+ assert_raises_rpc_error(-8, "Parameter args specified multiple times", self.nodes[0].cli("-named", "echo", "args=[0,1,2,3]", "4", "5", "6", ).send_cli)
I’m not sure if it’s worth changing, but seeing this test made me realize this error message could be confusing feedback to a user? They’re not specifying args multiple times explicitly - the fact that it’s labelled args internally may not be immediately obvious? But also maybe it’s a bit unnecessary to add special error handling for the args case.
re: #26628 (review)
I’m not sure if it’s worth changing, but seeing this test made me realize this error message could be confusing feedback to a user? They’re not specifying args multiple times explicitly - the fact that it’s labelled args internally may not be immediately obvious? But also maybe it’s a bit unnecessary to add special error handling for the args case.
Practically speaking, I don’t think this is an error case any human is going to see. It’s a corner case Marco found in #19762 (review) that was triggering unintended behavior in a newly added feature. Even if a bitcoin-cli user does manage to trigger this, I think the error message is accurate and helpful enough for finding the problem. I wouldn’t object to making a change here if there is a specific suggestion, but I also think the current behavior is ok, and there is room to add a special message later if the generic message turns out to be confusing.
