diff --git a/Makefile b/Makefile index 1ff408ea8..d53559ddf 100644 --- a/Makefile +++ b/Makefile @@ -324,7 +324,13 @@ check-tmpctx: check-discouraged-functions: @if git grep -E "[^a-z_/](fgets|fputs|gets|scanf|sprintf)\(" -- "*.c" "*.h" ":(exclude)ccan/"; then exit 1; fi -check-source: check-makefile check-source-bolt check-whitespace check-markdown check-spelling check-python check-includes check-cppcheck check-shellcheck check-setup_locale check-tmpctx check-discouraged-functions +# Don't access amount_msat and amount_sat members directly without a good reason +# since it risks overflow. +check-amount-access: + @! (git grep -nE "(->|\.)(milli)?satoshis" -- "*.c" "*.h" ":(exclude)common/amount.*" ":(exclude)*/test/*" | grep -v '/* Raw:') + @! git grep -nE "\\(struct amount_(m)?sat\\)" -- "*.c" "*.h" ":(exclude)common/amount.*" ":(exclude)*/test/*" + +check-source: check-makefile check-source-bolt check-whitespace check-markdown check-spelling check-python check-includes check-cppcheck check-shellcheck check-setup_locale check-tmpctx check-discouraged-functions check-amount-access full-check: check check-source diff --git a/bitcoin/pullpush.c b/bitcoin/pullpush.c index 1831a4641..baa94d501 100644 --- a/bitcoin/pullpush.c +++ b/bitcoin/pullpush.c @@ -29,7 +29,7 @@ void push_le64(u64 v, void push_amount_sat(struct amount_sat v, void (*push)(const void *, size_t, void *), void *pushp) { - push_le64(v.satoshis, push, pushp); + push_le64(v.satoshis, push, pushp); /* Raw: low-level helper */ } void push_varint_blob(const tal_t *blob, diff --git a/bitcoin/tx.c b/bitcoin/tx.c index 5a6c0492a..c0b2f8275 100644 --- a/bitcoin/tx.c +++ b/bitcoin/tx.c @@ -366,7 +366,7 @@ static struct amount_sat pull_amount_sat(const u8 **cursor, size_t *max) { struct amount_sat sat; - sat.satoshis = pull_value(cursor, max); + sat.satoshis = pull_value(cursor, max); /* Raw: low-level helper */ return sat; } diff --git a/channeld/commit_tx.c b/channeld/commit_tx.c index d6e0ec614..a8c359a0b 100644 --- a/channeld/commit_tx.c +++ b/channeld/commit_tx.c @@ -165,7 +165,7 @@ struct bitcoin_tx *commit_tx(const tal_t *ctx, ok &= amount_sat_add(&out, out, amount_msat_to_sat_round_down(other_pay)); assert(ok); SUPERVERBOSE("# actual commitment transaction fee = %"PRIu64"\n", - funding.satoshis - out.satoshis); + funding.satoshis - out.satoshis); /* Raw: test output */ } #endif diff --git a/channeld/full_channel.c b/channeld/full_channel.c index 50333aebc..071cca822 100644 --- a/channeld/full_channel.c +++ b/channeld/full_channel.c @@ -445,7 +445,7 @@ static enum channel_add_err add_htlc(struct channel *channel, } else fee = AMOUNT_SAT(0); - assert((s64)fee.satoshis >= 0); + assert((s64)fee.satoshis >= 0); /* Raw: explicit signedness test */ if (enforce_aggregate_limits) { /* Figure out what balance sender would have after applying all @@ -788,7 +788,7 @@ u32 approx_max_feerate(const struct channel *channel) channel->config[!channel->funder].channel_reserve)) avail = AMOUNT_SAT(0); - return avail.satoshis / weight * 1000; + return avail.satoshis / weight * 1000; /* Raw: once-off reverse feerate*/ } bool can_funder_afford_feerate(const struct channel *channel, u32 feerate_per_kw) diff --git a/closingd/closingd.c b/closingd/closingd.c index 00da9b14a..7b82fc625 100644 --- a/closingd/closingd.c +++ b/closingd/closingd.c @@ -463,7 +463,7 @@ static struct amount_sat adjust_offer(struct crypto_state *cs, type_to_string(tmpctx, struct amount_sat, &feerange->max)); - avg.satoshis /= 2; + avg.satoshis /= 2; /* Raw: average calculation */ return avg; } diff --git a/common/bolt11.c b/common/bolt11.c index a56a11bf7..ccbaf181c 100644 --- a/common/bolt11.c +++ b/common/bolt11.c @@ -558,7 +558,7 @@ struct bolt11 *bolt11_decode(const tal_t *ctx, const char *str, * amount required for payment. */ b11->msat = tal(b11, struct amount_msat); - b11->msat->millisatoshis = amount * m10 / 10; + b11->msat->millisatoshis = amount * m10 / 10; /* Raw: raw amount multiplier calculation */ } /* BOLT #11: @@ -889,7 +889,7 @@ char *bolt11_encode_(const tal_t *ctx, */ if (b11->msat) { char postfix; - u64 msat = b11->msat->millisatoshis; + u64 msat = b11->msat->millisatoshis; /* Raw: best-multiplier calc */ if (msat % MSAT_PER_BTC == 0) { postfix = '\0'; amount = msat / MSAT_PER_BTC; diff --git a/devtools/bolt11-cli.c b/devtools/bolt11-cli.c index c9815e1a6..49095af55 100644 --- a/devtools/bolt11-cli.c +++ b/devtools/bolt11-cli.c @@ -106,7 +106,7 @@ int main(int argc, char *argv[]) tal_hexstr(ctx, &b11->payment_hash, sizeof(b11->payment_hash))); printf("min_final_cltv_expiry: %u\n", b11->min_final_cltv_expiry); if (b11->msat) { - printf("msatoshi: %"PRIu64"\n", b11->msat->millisatoshis); + printf("msatoshi: %"PRIu64"\n", b11->msat->millisatoshis); /* Raw: raw int for backwards compat */ printf("amount_msat: %s\n", type_to_string(tmpctx, struct amount_msat, b11->msat)); } diff --git a/devtools/onion.c b/devtools/onion.c index 2707b015f..7c8cf08b8 100644 --- a/devtools/onion.c +++ b/devtools/onion.c @@ -38,7 +38,7 @@ static void do_generate(int argc, char **argv) hops_data[i].realm = i; memset(&hops_data[i].channel_id, i, sizeof(hops_data[i].channel_id)); - hops_data[i].amt_forward.millisatoshis = i; + hops_data[i].amt_forward.millisatoshis = i; /* Raw: test code */ hops_data[i].outgoing_cltv = i; fprintf(stderr, "Hopdata %d: %s\n", i, tal_hexstr(NULL, &hops_data[i], sizeof(hops_data[i]))); } diff --git a/gossipd/routing.c b/gossipd/routing.c index 393ca737c..8e6e9c89d 100644 --- a/gossipd/routing.c +++ b/gossipd/routing.c @@ -346,10 +346,10 @@ static WARN_UNUSED_RESULT bool risk_add_fee(struct amount_msat *risk, double r; /* Won't overflow on add, just lose precision */ - r = 1.0 + riskfactor * delay * msat.millisatoshis + risk->millisatoshis; + r = 1.0 + riskfactor * delay * msat.millisatoshis + risk->millisatoshis; /* Raw: to double */ if (r > UINT64_MAX) return false; - risk->millisatoshis = r; + risk->millisatoshis = r; /* Raw: from double */ return true; } @@ -406,7 +406,7 @@ static void bfg_one_edge(struct node *node, c->base_fee, c->proportional_fee)) continue; - if (!fuzz_fee(&fee.millisatoshis, fee_scale)) + if (!fuzz_fee(&fee.millisatoshis, fee_scale)) /* Raw: double manipulation */ continue; if (!amount_msat_add(&requiredcap, node->bfg[h].total, fee)) diff --git a/lightningd/bitcoind.c b/lightningd/bitcoind.c index 5292e0c25..d88181897 100644 --- a/lightningd/bitcoind.c +++ b/lightningd/bitcoind.c @@ -558,7 +558,7 @@ static bool process_gettxout(struct bitcoin_cli *bcli) bcli_args(tmpctx, bcli), (int)bcli->output_bytes, bcli->output); - if (!json_to_bitcoin_amount(bcli->output, valuetok, &out.amount.satoshis)) + if (!json_to_bitcoin_amount(bcli->output, valuetok, &out.amount.satoshis)) /* Raw: talking to bitcoind */ fatal("%s: had bad value (%.*s)?", bcli_args(tmpctx, bcli), (int)bcli->output_bytes, bcli->output); diff --git a/lightningd/invoice.c b/lightningd/invoice.c index 1e0097580..d4ed4aa64 100644 --- a/lightningd/invoice.c +++ b/lightningd/invoice.c @@ -198,9 +198,9 @@ static struct route_info **select_inchan(const tal_t *ctx, } /* Avoid divide-by-zero corner case. */ - wsum += excess.millisatoshis + 1; + wsum += excess.millisatoshis + 1; /* Raw: rand select */ if (pseudorand(1ULL << 32) - <= ((excess.millisatoshis + 1) << 32) / wsum) + <= ((excess.millisatoshis + 1) << 32) / wsum) /* Raw: rand select */ r = &inchans[i]; } @@ -289,9 +289,11 @@ static void gossipd_incoming_channels_reply(struct subd *gossipd, /* Warn if there's not sufficient incoming capacity. */ if (tal_count(info->b11->routes) == 0) { log_unusual(info->cmd->ld->log, - "invoice: insufficient incoming capacity for %"PRIu64 - " msatoshis%s", - info->b11->msat ? info->b11->msat->millisatoshis : 0, + "invoice: insufficient incoming capacity for %s%s", + info->b11->msat + ? type_to_string(tmpctx, struct amount_msat, + info->b11->msat) + : "0", any_offline ? " (among currently connected peers)" : ""); diff --git a/lightningd/json.c b/lightningd/json.c index 8fae0c9ce..70fdbaf21 100644 --- a/lightningd/json.c +++ b/lightningd/json.c @@ -335,7 +335,7 @@ void json_add_amount_msat(struct json_stream *result, const char *rawfieldname, const char *msatfieldname) { - json_add_u64(result, rawfieldname, msat.millisatoshis); + json_add_u64(result, rawfieldname, msat.millisatoshis); /* Raw: low-level helper */ json_add_member(result, msatfieldname, "\"%s\"", type_to_string(tmpctx, struct amount_msat, &msat)); } @@ -346,7 +346,7 @@ void json_add_amount_sat(struct json_stream *result, const char *msatfieldname) { struct amount_msat msat; - json_add_u64(result, rawfieldname, sat.satoshis); + json_add_u64(result, rawfieldname, sat.satoshis); /* Raw: low-level helper */ if (amount_sat_to_msat(&msat, sat)) json_add_member(result, msatfieldname, "\"%s\"", type_to_string(tmpctx, struct amount_msat, &msat)); diff --git a/lightningd/onchain_control.c b/lightningd/onchain_control.c index bd715a279..ec92607dd 100644 --- a/lightningd/onchain_control.c +++ b/lightningd/onchain_control.c @@ -464,7 +464,7 @@ enum watch_result onchaind_funding_spent(struct channel *channel, return KEEP_WATCHING; } - feerate = fee.satoshis / measure_tx_weight(tx); + feerate = fee.satoshis / measure_tx_weight(tx); /* Raw: reverse feerate extraction */ if (feerate < feerate_floor()) feerate = feerate_floor(); } diff --git a/lightningd/pay.c b/lightningd/pay.c index 9ec9bf972..f879f2830 100644 --- a/lightningd/pay.c +++ b/lightningd/pay.c @@ -811,11 +811,9 @@ static struct command_result *json_sendpay(struct command *cmd, /* if not: msatoshi <= finalhop.amount <= 2 * msatoshi, fail. */ if (msat) { - struct amount_msat two_times_final; + struct amount_msat limit = route[routetok->size-1].amount; - two_times_final.millisatoshis - = route[routetok->size-1].amount.millisatoshis * 2; - if (amount_msat_less(*msat, route[routetok->size-1].amount)) + if (amount_msat_less(*msat, limit)) return command_fail(cmd, JSONRPC2_INVALID_PARAMS, "msatoshi %s less than final %s", type_to_string(tmpctx, @@ -824,7 +822,8 @@ static struct command_result *json_sendpay(struct command *cmd, type_to_string(tmpctx, struct amount_msat, &route[routetok->size-1].amount)); - if (amount_msat_greater(*msat, two_times_final)) + limit.millisatoshis *= 2; /* Raw: sanity check */ + if (amount_msat_greater(*msat, limit)) return command_fail(cmd, JSONRPC2_INVALID_PARAMS, "msatoshi %s more than twice final %s", type_to_string(tmpctx, diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index 0081bc88e..bfa3cf2af 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -538,11 +538,11 @@ static void json_add_channel(struct lightningd *ld, if (channel->funder == LOCAL) { json_add_u64(response, pubkey_to_hexstr(tmpctx, &p->id), 0); json_add_u64(response, pubkey_to_hexstr(tmpctx, &ld->id), - channel->funding.satoshis * 1000); + channel->funding.satoshis * 1000); /* Raw: raw JSON field */ } else { json_add_u64(response, pubkey_to_hexstr(tmpctx, &ld->id), 0); json_add_u64(response, pubkey_to_hexstr(tmpctx, &p->id), - channel->funding.satoshis * 1000); + channel->funding.satoshis * 1000); /* Raw: raw JSON field */ } json_object_end(response); diff --git a/openingd/openingd.c b/openingd/openingd.c index dd25f498c..1c263f906 100644 --- a/openingd/openingd.c +++ b/openingd/openingd.c @@ -302,8 +302,8 @@ static bool check_config_bounds(struct state *state, /* We always set channel_reserve_satoshis to 1%, rounded up. */ static void set_reserve(struct state *state) { - state->localconf.channel_reserve.satoshis - = (state->funding.satoshis + 99) / 100; + state->localconf.channel_reserve.satoshis /* Raw: rounding. */ + = (state->funding.satoshis + 99) / 100; /* Raw: rounding. */ /* BOLT #2: * diff --git a/plugins/pay.c b/plugins/pay.c index dd851bc4e..72342799a 100644 --- a/plugins/pay.c +++ b/plugins/pay.c @@ -447,7 +447,7 @@ static struct command_result *getroute_done(struct command *cmd, * in feepercent will be like 3.0000..(some dots)..1 % - 3.0 %. * That loss will not be representable in double. So, it's Okay to * cast u64 to double for feepercent calculation. */ - feepercent = ((double)fee.millisatoshis) * 100.0 / ((double) pc->msat.millisatoshis); + feepercent = ((double)fee.millisatoshis) * 100.0 / ((double) pc->msat.millisatoshis); /* Raw: fee double manipulation */ if (amount_msat_greater(fee, pc->exemptfee) && feepercent > pc->maxfeepercent) { @@ -468,7 +468,7 @@ static struct command_result *getroute_done(struct command *cmd, /* Try excluding most fee-charging channel (unless it's in * routeboost). */ - charger = find_worst_channel(buf, t, "msatoshi", pc->msat.millisatoshis); + charger = find_worst_channel(buf, t, "msatoshi", pc->msat.millisatoshis); /* Raw: shared function needs u64 */ if (maybe_exclude(pc, buf, charger)) { return start_pay_attempt(cmd, pc, "Excluded expensive channel %s", @@ -1030,7 +1030,7 @@ static struct command_result *handle_paystatus(struct command *cmd, " 'amount_msat': '%s', " " 'destination': '%s'", ps->bolt11, - ps->msat.millisatoshis, + ps->msat.millisatoshis, /* Raw: JSON */ type_to_string(tmpctx, struct amount_msat, &ps->msat), ps->dest); if (ps->desc) diff --git a/wallet/db.c b/wallet/db.c index 897ea1fb7..2b48a4afa 100644 --- a/wallet/db.c +++ b/wallet/db.c @@ -948,7 +948,7 @@ struct amount_msat sqlite3_column_amount_msat(sqlite3_stmt *stmt, int col) { struct amount_msat msat; - msat.millisatoshis = sqlite3_column_int64(stmt, col); + msat.millisatoshis = sqlite3_column_int64(stmt, col); /* Raw: low level function */ return msat; } @@ -956,18 +956,18 @@ struct amount_sat sqlite3_column_amount_sat(sqlite3_stmt *stmt, int col) { struct amount_sat sat; - sat.satoshis = sqlite3_column_int64(stmt, col); + sat.satoshis = sqlite3_column_int64(stmt, col); /* Raw: low level function */ return sat; } void sqlite3_bind_amount_msat(sqlite3_stmt *stmt, int col, struct amount_msat msat) { - sqlite3_bind_int64(stmt, col, msat.millisatoshis); + sqlite3_bind_int64(stmt, col, msat.millisatoshis); /* Raw: low level function */ } void sqlite3_bind_amount_sat(sqlite3_stmt *stmt, int col, struct amount_sat sat) { - sqlite3_bind_int64(stmt, col, sat.satoshis); + sqlite3_bind_int64(stmt, col, sat.satoshis); /* Raw: low level function */ } diff --git a/wallet/wallet.c b/wallet/wallet.c index 9bbb62d16..f611b9ef5 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -787,7 +787,7 @@ void wallet_channel_stats_incr_x(struct wallet *w, " , %s = COALESCE(%s, 0) + %"PRIu64"" " WHERE id = %"PRIu64";", payments_stat, payments_stat, - msatoshi_stat, msatoshi_stat, msat.millisatoshis, + msatoshi_stat, msatoshi_stat, msat.millisatoshis, /* Raw: db access */ cdbid); sqlite3_stmt *stmt = db_prepare(w->db, qry); db_exec_prepared(w->db, stmt); diff --git a/wire/fromwire.c b/wire/fromwire.c index d542901b4..4490fe11d 100644 --- a/wire/fromwire.c +++ b/wire/fromwire.c @@ -276,7 +276,7 @@ struct amount_msat fromwire_amount_msat(const u8 **cursor, size_t *max) { struct amount_msat msat; - msat.millisatoshis = fromwire_u64(cursor, max); + msat.millisatoshis = fromwire_u64(cursor, max); /* Raw: primitive */ return msat; } @@ -284,7 +284,7 @@ struct amount_sat fromwire_amount_sat(const u8 **cursor, size_t *max) { struct amount_sat sat; - sat.satoshis = fromwire_u64(cursor, max); + sat.satoshis = fromwire_u64(cursor, max); /* Raw: primitive */ return sat; } diff --git a/wire/towire.c b/wire/towire.c index 5de803e4a..9b71d1193 100644 --- a/wire/towire.c +++ b/wire/towire.c @@ -186,10 +186,10 @@ void towire_siphash_seed(u8 **pptr, const struct siphash_seed *seed) void towire_amount_msat(u8 **pptr, const struct amount_msat msat) { - towire_u64(pptr, msat.millisatoshis); + towire_u64(pptr, msat.millisatoshis); /* Raw: primitive */ } void towire_amount_sat(u8 **pptr, const struct amount_sat sat) { - towire_u64(pptr, sat.satoshis); + towire_u64(pptr, sat.satoshis); /* Raw: primitive */ }