Skip to content
Permalink
Browse files

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 <rusty@rustcorp.com.au>
  • Loading branch information...
rustyrussell committed Feb 21, 2019
1 parent e96861f commit fa26fb17cf29ae896b2332c65486b965e53094fa
@@ -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

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

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

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

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

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

@@ -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:
*
@@ -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)
@@ -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 */
}
@@ -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);
@@ -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;
}

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

0 comments on commit fa26fb1

Please sign in to comment.
You can’t perform that action at this time.