Keep HMAC case consistent

[OrfeasLitos]

Change HMAC to lowercase in packet description, as it later appears in
lowercase. The uppercase version is used in the hops_data, so this
change helps avoid confusion.

[sstone]

It would then be consistent to also use hmac instead of HMAC in the description of hops_data.

[OrfeasLitos]

I couldn't find the exact spot, can you tell me the line?

[sstone]

In the description of hops_data lines 171/175 we still have HMAC, it would be more consistent to use hmac here as well.

[OrfeasLitos]

I was suggesting to use hmac for the single last HMAC at the end of an onion_packet and HMAC for each of the many HMACs that appear within hops_data. Do you suggest to change all HMAC occurrences to hmac instead?

Observe that the HMAC of lines 171 and 175 is of the kind that is within hops_data, hence according to the proposed convention it should remain uppercase.

[sstone]

Yes I'm suggesting it's consistent to use lowercase here as well (we use lowercase for field names everywhere else).

[OrfeasLitos]

In that case, would you consider some other way for differentiating the two kinds of HMAC, e.g. hop_hmac and packet_hmac? This is consistent with lowercase field names and helps differentiate the two kinds of HMAC, thus reducing confusion. Sorry for my insistence, but the point of this PR is to help the reader tell the two kinds of HMAC apart easily.

[sstone]

Sorry for my insistence, but the point of this PR is to help the reader tell the two kinds of HMAC apart easily.

Yes but I don't think it's needed, though fixing naming inconsistencies may be useful. This part of the spec is fairly technical and probably harder to understand than other BOLTs but I don't think that people trying to implement it would get confused by the packet/hop data HMAC fields having the same name.

In other words, for me using lowercase for the hmac fields -> ACK, having hmac for one field and HMAC for the other -> NACK

[OrfeasLitos]

Ok, I just changed all fields to lowercase hmac.

[TheBlueMatt]

IRC discussion indicated most folks prefer upper-case for cryptographic primitives, especially when they're acronyms.

[OrfeasLitos]

Fixed

[OrfeasLitos]

All HMAC are uppercase now, ready to merge

[niftynei]

also NACK'ing this; is inconsistent with all other field names.

[t-bast]

NACK too, I prefer consistency here.

[OrfeasLitos]

@niftynei @t-bast what do you suggest? Summarizing options:

• All HMAC fields lowercase
• All HMAC fields uppercase
• Lowercase for HMAC at the end of onion_packet, uppercase for HMAC in hop_payload (or vice-versa)
• Rename fields to onion_hmac and hop_hmac (distinguishes two HMACs, keeps lowercase)
• Any other ideas?

[t-bast]

I would not change anything (keep all hmac fields lowercase). I honestly didn't find this confusing when implementing.

[OrfeasLitos]

Currently not all fields are lowercase. Initially this PR was just for lowercasing the HMAC of line (currently) 135, to make it consistent with line 149. Then we realized that there is a bigger inconsistency, e.g. line 160.

Given that a change is needed, we may as well choose something unambiguous for field names (I got confused when I first read this). My recommendation is to change the field names to onion_hmac and hop_hmac so that it is clear which HMAC authenticates what.

[t-bast]

Indeed, I didn't notice that in some struct fields it's upper-case.
My personal choice would be to have it lower-case in message types to be consistent with other fields in those types.

I don't like renaming those, I think it's more confusing: these macs always authenticate the whole onion. The outer one authenticates the onion before decryption, the inner one authenticates the new onion you obtain after decryption (the onion you will forward to the next node). Does that make sense?

[OrfeasLitos]

Good point, those names would be confusing. I therefore support lowercasing all fields. @TheBlueMatt maybe the objections from IRC are now obsolete? Or are there still folks that prefer HMAC as a field name? (Note that I'm not referring to the use of the word outside backticks, there it will be kept uppercase "HMAC".)

[OrfeasLitos]

I changed all "HMAC"s to "hmac", ready to merge.

[t-bast]

Thanks, ACK e3fc992.