Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

amount_sat/amount_msat audit results #3904

Merged
merged 4 commits into from
Aug 6, 2020
Merged

amount_sat/amount_msat audit results #3904

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d80f652
amount: add helper to convert u64 sats to amount_sat type
niftynei Aug 5, 2020
a391c23
amount: add amount_msat and amount_sat initializers.
rustyrussell Aug 5, 2020
8d42bc7
amount: use initializers everywhere.
rustyrussell Aug 5, 2020
4c2dcc7
amount: add amount_msat_scale, amount_msat_ratio, amount_{msat,sat}_div
rustyrussell Aug 5, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions closingd/closingd.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -540,17 +540,16 @@ adjust_offer(struct per_peer_state *pps, const struct channel_id *channel_id,
* one from our previous proposal. So, if the user requested a
* step of 1 satoshi at a time we should just return our end of
* the range from this function. */
amount_msat_from_u64(&step_msat,
(fee_negotiation_step - 1) * MSAT_PER_SAT);
step_msat = amount_msat((fee_negotiation_step - 1)
* MSAT_PER_SAT);
} else {
/* fee_negotiation_step is e.g. 20 to designate 20% from
* range_len (which is in satoshi), so:
* range_len * fee_negotiation_step / 100 [sat]
* is equivalent to:
* range_len * fee_negotiation_step * 10 [msat] */
amount_msat_from_u64(&step_msat,
range_len.satoshis /* Raw: % calc */ *
fee_negotiation_step * 10);
step_msat = amount_msat(range_len.satoshis /* Raw: % calc */ *
fee_negotiation_step * 10);
}

step_sat = amount_msat_to_sat_round_down(step_msat);
Expand Down
17 changes: 10 additions & 7 deletions common/amount.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -401,17 +401,20 @@ bool amount_msat_to_u32(struct amount_msat msat, u32 *millisatoshis)
return true;
}

void amount_msat_from_u64(struct amount_msat *msat, u64 millisatoshis)
struct amount_msat amount_msat(u64 millisatoshis)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this better than the void foo(out* out, int input) style. This is in fact what I thought AMOUNT_SAT/AMOUNT_MSAT did back when they were introduced, and was disappointed to learn they were just constexpr, boo.

The compiler might be able to optimize these better if we static inline these in the headers.

{
msat->millisatoshis = millisatoshis;
struct amount_msat msat;

msat.millisatoshis = millisatoshis;
return msat;
}

WARN_UNUSED_RESULT bool amount_msat_from_sat_u64(struct amount_msat *msat, u64 satoshis)
struct amount_sat amount_sat(u64 satoshis)
{
if (mul_overflows_u64(satoshis, MSAT_PER_SAT))
return false;
msat->millisatoshis = satoshis * MSAT_PER_SAT;
return true;
struct amount_sat sat;

sat.satoshis = satoshis;
return sat;
}

bool amount_msat_fee(struct amount_msat *fee,
Expand Down
8 changes: 4 additions & 4 deletions common/amount.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ struct amount_asset {
#define AMOUNT_SAT(constant) \
((struct amount_sat){(constant) + AMOUNT_MUST_BE_CONST(constant)})

/* We do sometimes need to import from raw types, eg. wally or wire fmt */
struct amount_msat amount_msat(u64 millisatoshis);
struct amount_sat amount_sat(u64 satoshis);

/* You may not always be able to convert satoshis->millisatoshis. */
WARN_UNUSED_RESULT bool amount_sat_to_msat(struct amount_msat *msat,
struct amount_sat sat);
Expand Down Expand Up @@ -128,10 +132,6 @@ struct amount_sat amount_asset_to_sat(struct amount_asset *asset);
WARN_UNUSED_RESULT bool amount_msat_to_u32(struct amount_msat msat,
u32 *millisatoshis);

/* Programatically initialize from various types */
void amount_msat_from_u64(struct amount_msat *msat, u64 millisatoshis);
WARN_UNUSED_RESULT bool amount_msat_from_sat_u64(struct amount_msat *msat, u64 satoshis);

/* Common operation: what is the HTLC fee for given feerate? Can overflow! */
WARN_UNUSED_RESULT bool amount_msat_fee(struct amount_msat *fee,
struct amount_msat amt,
Expand Down
2 changes: 1 addition & 1 deletion common/onion.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ struct onion_payload *onion_decode(const tal_t *ctx,
if (!tlv->amt_to_forward || !tlv->outgoing_cltv_value)
goto fail;

amount_msat_from_u64(&p->amt_to_forward, *tlv->amt_to_forward);
p->amt_to_forward = amount_msat(*tlv->amt_to_forward);
p->outgoing_cltv = *tlv->outgoing_cltv_value;

/* BOLT #4:
Expand Down
6 changes: 3 additions & 3 deletions common/test/run-sphinx.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ bool amount_asset_is_main(struct amount_asset *asset UNNEEDED)
/* Generated stub for amount_asset_to_sat */
struct amount_sat amount_asset_to_sat(struct amount_asset *asset UNNEEDED)
{ fprintf(stderr, "amount_asset_to_sat called!\n"); abort(); }
/* Generated stub for amount_msat */
struct amount_msat amount_msat(u64 millisatoshis UNNEEDED)
{ fprintf(stderr, "amount_msat called!\n"); abort(); }
/* Generated stub for amount_msat_eq */
bool amount_msat_eq(struct amount_msat a UNNEEDED, struct amount_msat b UNNEEDED)
{ fprintf(stderr, "amount_msat_eq called!\n"); abort(); }
/* Generated stub for amount_msat_from_u64 */
void amount_msat_from_u64(struct amount_msat *msat UNNEEDED, u64 millisatoshis UNNEEDED)
{ fprintf(stderr, "amount_msat_from_u64 called!\n"); abort(); }
/* Generated stub for amount_sat_add */
bool amount_sat_add(struct amount_sat *val UNNEEDED,
struct amount_sat a UNNEEDED,
Expand Down
3 changes: 0 additions & 3 deletions gossipd/test/run-bench-find_route.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,6 @@ bool nannounce_different(struct gossip_store *gs UNNEEDED,
/* Generated stub for notleak_ */
void *notleak_(const void *ptr UNNEEDED, bool plus_children UNNEEDED)
{ fprintf(stderr, "notleak_ called!\n"); abort(); }
/* Generated stub for onion_type_name */
const char *onion_type_name(int e UNNEEDED)
{ fprintf(stderr, "onion_type_name called!\n"); abort(); }
/* Generated stub for peer_supplied_good_gossip */
void peer_supplied_good_gossip(struct peer *peer UNNEEDED, size_t amount UNNEEDED)
{ fprintf(stderr, "peer_supplied_good_gossip called!\n"); abort(); }
Expand Down
3 changes: 0 additions & 3 deletions gossipd/test/run-check_channel_announcement.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,6 @@ struct oneshot *new_reltimer_(struct timers *timers UNNEEDED,
/* Generated stub for notleak_ */
void *notleak_(const void *ptr UNNEEDED, bool plus_children UNNEEDED)
{ fprintf(stderr, "notleak_ called!\n"); abort(); }
/* Generated stub for onion_type_name */
const char *onion_type_name(int e UNNEEDED)
{ fprintf(stderr, "onion_type_name called!\n"); abort(); }
/* Generated stub for peer_supplied_good_gossip */
void peer_supplied_good_gossip(struct peer *peer UNNEEDED, size_t amount UNNEEDED)
{ fprintf(stderr, "peer_supplied_good_gossip called!\n"); abort(); }
Expand Down
3 changes: 0 additions & 3 deletions gossipd/test/run-find_route-specific.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,6 @@ bool nannounce_different(struct gossip_store *gs UNNEEDED,
/* Generated stub for notleak_ */
void *notleak_(const void *ptr UNNEEDED, bool plus_children UNNEEDED)
{ fprintf(stderr, "notleak_ called!\n"); abort(); }
/* Generated stub for onion_type_name */
const char *onion_type_name(int e UNNEEDED)
{ fprintf(stderr, "onion_type_name called!\n"); abort(); }
/* Generated stub for peer_supplied_good_gossip */
void peer_supplied_good_gossip(struct peer *peer UNNEEDED, size_t amount UNNEEDED)
{ fprintf(stderr, "peer_supplied_good_gossip called!\n"); abort(); }
Expand Down
3 changes: 0 additions & 3 deletions gossipd/test/run-find_route.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,6 @@ bool nannounce_different(struct gossip_store *gs UNNEEDED,
/* Generated stub for notleak_ */
void *notleak_(const void *ptr UNNEEDED, bool plus_children UNNEEDED)
{ fprintf(stderr, "notleak_ called!\n"); abort(); }
/* Generated stub for onion_type_name */
const char *onion_type_name(int e UNNEEDED)
{ fprintf(stderr, "onion_type_name called!\n"); abort(); }
/* Generated stub for peer_supplied_good_gossip */
void peer_supplied_good_gossip(struct peer *peer UNNEEDED, size_t amount UNNEEDED)
{ fprintf(stderr, "peer_supplied_good_gossip called!\n"); abort(); }
Expand Down
3 changes: 0 additions & 3 deletions gossipd/test/run-overlong.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,6 @@ bool nannounce_different(struct gossip_store *gs UNNEEDED,
/* Generated stub for notleak_ */
void *notleak_(const void *ptr UNNEEDED, bool plus_children UNNEEDED)
{ fprintf(stderr, "notleak_ called!\n"); abort(); }
/* Generated stub for onion_type_name */
const char *onion_type_name(int e UNNEEDED)
{ fprintf(stderr, "onion_type_name called!\n"); abort(); }
/* Generated stub for peer_supplied_good_gossip */
void peer_supplied_good_gossip(struct peer *peer UNNEEDED, size_t amount UNNEEDED)
{ fprintf(stderr, "peer_supplied_good_gossip called!\n"); abort(); }
Expand Down
3 changes: 0 additions & 3 deletions gossipd/test/run-txout_failure.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,6 @@ bool nannounce_different(struct gossip_store *gs UNNEEDED,
/* Generated stub for notleak_ */
void *notleak_(const void *ptr UNNEEDED, bool plus_children UNNEEDED)
{ fprintf(stderr, "notleak_ called!\n"); abort(); }
/* Generated stub for onion_type_name */
const char *onion_type_name(int e UNNEEDED)
{ fprintf(stderr, "onion_type_name called!\n"); abort(); }
/* Generated stub for peer_supplied_good_gossip */
void peer_supplied_good_gossip(struct peer *peer UNNEEDED, size_t amount UNNEEDED)
{ fprintf(stderr, "peer_supplied_good_gossip called!\n"); abort(); }
Expand Down
4 changes: 2 additions & 2 deletions lightningd/opening_control.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -705,8 +705,8 @@ static void channel_config(struct lightningd *ld,
*max_to_self_delay = ld->config.locktime_max;

/* Take minimal effective capacity from config min_capacity_sat */
if (!amount_msat_from_sat_u64(min_effective_htlc_capacity,
ld->config.min_capacity_sat))
if (!amount_sat_to_msat(min_effective_htlc_capacity,
amount_sat(ld->config.min_capacity_sat)))
fatal("amount_msat overflow for config.min_capacity_sat");
/* Substract 2 * dust_limit, so fundchannel with min value is possible */
if (!amount_sat_to_msat(&dust_limit, chainparams->dust_limit))
Expand Down
5 changes: 1 addition & 4 deletions lightningd/test/run-find_my_abspath.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,6 @@ int unused_main(int argc, char *argv[]);
/* Generated stub for activate_peers */
void activate_peers(struct lightningd *ld UNNEEDED)
{ fprintf(stderr, "activate_peers called!\n"); abort(); }
/* Generated stub for add_plugin_dir */
char *add_plugin_dir(struct plugins *plugins UNNEEDED, const char *dir UNNEEDED,
bool error_ok UNNEEDED)
{ fprintf(stderr, "add_plugin_dir called!\n"); abort(); }
/* Generated stub for begin_topology */
void begin_topology(struct chain_topology *topo UNNEEDED)
{ fprintf(stderr, "begin_topology called!\n"); abort(); }
Expand Down Expand Up @@ -207,6 +203,7 @@ void plugins_init(struct plugins *plugins UNNEEDED)
struct plugins *plugins_new(const tal_t *ctx UNNEEDED, struct log_book *log_book UNNEEDED,
struct lightningd *ld UNNEEDED)
{ fprintf(stderr, "plugins_new called!\n"); abort(); }
/* Generated stub for plugins_set_builtin_plugins_dir */
void plugins_set_builtin_plugins_dir(struct plugins *plugins UNNEEDED,
const char *dir UNNEEDED)
{ fprintf(stderr, "plugins_set_builtin_plugins_dir called!\n"); abort(); }
Expand Down