Makefile: check for direct amount_sat/amount_msat access.

We need to do it in various places, but we shouldn't do it lightly:
the primitives are there to help us get overflow handling correct.

Signed-off-by: Rusty Russell <[email protected]>

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
 

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,

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;
 }
 

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
 

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)

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;
 }
 

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;

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));
 	}

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])));
 	}

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))

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);

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)" : "");
 

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));

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();
 	}

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,

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);
 

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:
 	 *

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)

wallet/db.c

@@ -948,26 +948,26 @@ 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;
 }
 
 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 */
 }

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);

wire/fromwire.c

@@ -276,15 +276,15 @@ 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;
 }
 
 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;
 }
 

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 */
 }