selfcustody / krux

Open-source signing device firmware for Bitcoin
https://selfcustody.github.io/krux/
Other
179 stars 36 forks source link

[minor issue] Index button of the previous addresses is not correct? #283

Closed aglkm closed 11 months ago

aglkm commented 11 months ago

Shouldn't we need to correct the index of the previous addresses?

Change the following:

def list_address_type(self, addr_type=0): ... "%d..%d" % (num_checked - max_addresses, num_checked),

to:

"%d..%d" % (num_checked - max_addresses + 1, num_checked),

krux_issue01

jdlcdl commented 11 months ago

Just here to chime in that "Index starts at 0... while Numbers start at 1". I'm sort of making that up... but just for consistency and terminology, "index" sounds computer-ish so that I remember it starts at 0, and "number" sounds human so that we start counting at 1. It's growing on me that the first address (with a derivation path that ends in /0) is "1."

... and I agree with @aglkm above!

odudex commented 11 months ago

Good catch! Wouldn't you like to make a PR? I think we should keep the numbers, starting at 1, as we also refer to as first, second address etc. and just make the change @aglkm suggested.

aglkm commented 11 months ago

Sure. I will add a PR tomorrow.

aglkm commented 11 months ago

Code changes added here https://github.com/odudex/krux/pull/36