Merge bitcoin/bitcoin#25551: refactor: Throw exception on invalid Univalue pushes over silent ignore

fa277cd55d univalue: Throw exception on invalid pushes over silent ignore (MacroFake)
ccccc17b91 refactor: Default options in walletcreatefundedpsbt to VOBJ instead of VNULL (MacroFake)

Pull request description:

  The return value of the `push*` helpers is never used, but important to determine if the operation was successful. One way to fix this would be to add the "nodiscard" attribute. However, this would make the code (and this diff) overly verbose for no reason.

  So fix it by removing the never used return value. Also, fail verbosely in case of a programming mistake.

ACKs for top commit:
  furszy:
    code ACK fa277cd5

Tree-SHA512: ef212a5bf5ae6bbad20acc4dafa3715521e81544185988d1eab724f440e4864a27e686aff51d5bc51b3017892c2eb8e577bcb8f37e8ddbaa0d8833bb622f2f9c
This commit is contained in:
fanquake 2022-07-15 16:31:08 +01:00
commit a969b2fcd3
No known key found for this signature in database
GPG key ID: 2EEB9F5CC09526C1
4 changed files with 50 additions and 49 deletions

View file

@ -82,14 +82,14 @@ public:
bool isArray() const { return (typ == VARR); }
bool isObject() const { return (typ == VOBJ); }
bool push_back(const UniValue& val);
bool push_backV(const std::vector<UniValue>& vec);
void push_back(const UniValue& val);
void push_backV(const std::vector<UniValue>& vec);
template <class It>
bool push_backV(It first, It last);
void push_backV(It first, It last);
void __pushKV(const std::string& key, const UniValue& val);
bool pushKV(const std::string& key, const UniValue& val);
bool pushKVs(const UniValue& obj);
void pushKV(const std::string& key, const UniValue& val);
void pushKVs(const UniValue& obj);
std::string write(unsigned int prettyIndent = 0,
unsigned int indentLevel = 0) const;
@ -140,11 +140,10 @@ public:
};
template <class It>
bool UniValue::push_backV(It first, It last)
void UniValue::push_backV(It first, It last)
{
if (typ != VARR) return false;
if (typ != VARR) throw std::runtime_error{"JSON value is not an array as expected"};
values.insert(values.end(), first, last);
return true;
}
enum jtokentype {

View file

@ -108,53 +108,45 @@ bool UniValue::setObject()
return true;
}
bool UniValue::push_back(const UniValue& val_)
void UniValue::push_back(const UniValue& val_)
{
if (typ != VARR)
return false;
if (typ != VARR) throw std::runtime_error{"JSON value is not an array as expected"};
values.push_back(val_);
return true;
}
bool UniValue::push_backV(const std::vector<UniValue>& vec)
void UniValue::push_backV(const std::vector<UniValue>& vec)
{
if (typ != VARR)
return false;
if (typ != VARR) throw std::runtime_error{"JSON value is not an array as expected"};
values.insert(values.end(), vec.begin(), vec.end());
return true;
}
void UniValue::__pushKV(const std::string& key, const UniValue& val_)
{
if (typ != VOBJ) throw std::runtime_error{"JSON value is not an object as expected"};
keys.push_back(key);
values.push_back(val_);
}
bool UniValue::pushKV(const std::string& key, const UniValue& val_)
void UniValue::pushKV(const std::string& key, const UniValue& val_)
{
if (typ != VOBJ)
return false;
if (typ != VOBJ) throw std::runtime_error{"JSON value is not an object as expected"};
size_t idx;
if (findKey(key, idx))
values[idx] = val_;
else
__pushKV(key, val_);
return true;
}
bool UniValue::pushKVs(const UniValue& obj)
void UniValue::pushKVs(const UniValue& obj)
{
if (typ != VOBJ || obj.typ != VOBJ)
return false;
if (typ != VOBJ || obj.typ != VOBJ) throw std::runtime_error{"JSON value is not an object as expected"};
for (size_t i = 0; i < obj.keys.size(); i++)
__pushKV(obj.keys[i], obj.values.at(i));
return true;
}
void UniValue::getObjMap(std::map<std::string,UniValue>& kv) const

View file

@ -85,6 +85,16 @@ BOOST_AUTO_TEST_CASE(univalue_constructor)
BOOST_CHECK_EQUAL(v9.getValStr(), "zappa");
}
BOOST_AUTO_TEST_CASE(univalue_push_throw)
{
UniValue j;
BOOST_CHECK_THROW(j.push_back(1), std::runtime_error);
BOOST_CHECK_THROW(j.push_backV({1}), std::runtime_error);
BOOST_CHECK_THROW(j.__pushKV("k", 1), std::runtime_error);
BOOST_CHECK_THROW(j.pushKV("k", 1), std::runtime_error);
BOOST_CHECK_THROW(j.pushKVs({}), std::runtime_error);
}
BOOST_AUTO_TEST_CASE(univalue_typecheck)
{
UniValue v1;
@ -198,13 +208,13 @@ BOOST_AUTO_TEST_CASE(univalue_array)
UniValue arr(UniValue::VARR);
UniValue v((int64_t)1023LL);
BOOST_CHECK(arr.push_back(v));
arr.push_back(v);
std::string vStr("zippy");
BOOST_CHECK(arr.push_back(vStr));
arr.push_back(vStr);
const char *s = "pippy";
BOOST_CHECK(arr.push_back(s));
arr.push_back(s);
std::vector<UniValue> vec;
v.setStr("boing");
@ -213,13 +223,13 @@ BOOST_AUTO_TEST_CASE(univalue_array)
v.setStr("going");
vec.push_back(v);
BOOST_CHECK(arr.push_backV(vec));
arr.push_backV(vec);
BOOST_CHECK(arr.push_back((uint64_t) 400ULL));
BOOST_CHECK(arr.push_back((int64_t) -400LL));
BOOST_CHECK(arr.push_back((int) -401));
BOOST_CHECK(arr.push_back(-40.1));
BOOST_CHECK(arr.push_back(true));
arr.push_back(uint64_t{400ULL});
arr.push_back(int64_t{-400LL});
arr.push_back(int{-401});
arr.push_back(-40.1);
arr.push_back(true);
BOOST_CHECK_EQUAL(arr.empty(), false);
BOOST_CHECK_EQUAL(arr.size(), 10);
@ -260,39 +270,39 @@ BOOST_AUTO_TEST_CASE(univalue_object)
strKey = "age";
v.setInt(100);
BOOST_CHECK(obj.pushKV(strKey, v));
obj.pushKV(strKey, v);
strKey = "first";
strVal = "John";
BOOST_CHECK(obj.pushKV(strKey, strVal));
obj.pushKV(strKey, strVal);
strKey = "last";
const char *cVal = "Smith";
BOOST_CHECK(obj.pushKV(strKey, cVal));
const char* cVal = "Smith";
obj.pushKV(strKey, cVal);
strKey = "distance";
BOOST_CHECK(obj.pushKV(strKey, (int64_t) 25));
obj.pushKV(strKey, int64_t{25});
strKey = "time";
BOOST_CHECK(obj.pushKV(strKey, (uint64_t) 3600));
obj.pushKV(strKey, uint64_t{3600});
strKey = "calories";
BOOST_CHECK(obj.pushKV(strKey, (int) 12));
obj.pushKV(strKey, int{12});
strKey = "temperature";
BOOST_CHECK(obj.pushKV(strKey, (double) 90.012));
obj.pushKV(strKey, double{90.012});
strKey = "moon";
BOOST_CHECK(obj.pushKV(strKey, true));
obj.pushKV(strKey, true);
strKey = "spoon";
BOOST_CHECK(obj.pushKV(strKey, false));
obj.pushKV(strKey, false);
UniValue obj2(UniValue::VOBJ);
BOOST_CHECK(obj2.pushKV("cat1", 9000));
BOOST_CHECK(obj2.pushKV("cat2", 12345));
obj2.pushKV("cat1", 9000);
obj2.pushKV("cat2", 12345);
BOOST_CHECK(obj.pushKVs(obj2));
obj.pushKVs(obj2);
BOOST_CHECK_EQUAL(obj.empty(), false);
BOOST_CHECK_EQUAL(obj.size(), 11);

View file

@ -1633,7 +1633,7 @@ RPCHelpMan walletcreatefundedpsbt()
}, true
);
UniValue options = request.params[3];
UniValue options{request.params[3].isNull() ? UniValue::VOBJ : request.params[3]};
CAmount fee;
int change_position;