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

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.

<!--e57a25ab6845829454e8d69fc972939a-->

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #25617 (refactor: univalue test cleanups by fanquake)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

Concept ACK

Would be good to add coverage for it with few BOOST_CHECK_THROW (one per changed method).

Thanks, done

cc @martinus @furszy want to take another look?

Rebased

code ACK fa277cd5

Non-blocking cosmetic nit:

Could probably unify all the throw calls with something like this:

static void ThrowIfNotEqual(UniValue::VType type, UniValue::VType expected_type)
{
    if (type != expected_type) throw std::runtime_error{"JSON value is not an " + std::string(uvTypeName(type)) + " as expected"};
}

Then just call ThrowIfNotEqual(typ, VARR) or ThrowIfNotEqual(typ, VOBJ) at the beginning of each function.

Could probably unify all the throw calls with something like this:

Thanks, will do in a non-refactoring follow-up