Importmulti api is confusing in a way that could lead to funds loss.

[gmaxwell]

Our normal import interfaces introduce keys with an effective timestamp of 0.

Importmulti introduces keys with a default effective timestamp of 'now'.

While documented it is highly surprising, especially because it contradicts the default on the rescan argument:

"rescan": <false>,         (boolean, optional, default: true) Stating if should rescan the blockchain after all imports

This means you can import a key and reasonably expect that it was rescanned ("I even set rescan true!") but have it not be rescanned, causing you to think the key had been unused and then you give it away and or destroy it and lose funds.

There are a number of other bugs in importmulti (mishandles pruning, needlessly forces the wallet to be unlocked when not importing private keys, etc.) which I am fixing, but I need feedback on this API issue.

Should the default become zero, with -1 special cased as 'now' or make timestamp mandatory? default it to the prune depth??

Should the result also tell us what it rescanned?

[morcos]

@gmaxwell I also found this surprising. My first inclination was that we should change it, but then it means if you ever leave off a timestamp, you're stuck rescanning the whole chain.

Perhaps with proper documentation we could have 3 states.
rescan: false - no rescan
no rescan option set - rescan starts at the minimum of only the provided timestamps (current behavior)
rescan: true - rescan starts at genesis unless all timestamps are provided and then starts at the minimum of the timestamps.

I only wish it could tell you what it was planning to rescan before it did it, so you could pull the plug on your computer if you got it wrong.

[gmaxwell]

One option is that if it concludes it should rescan everything, it could fail and force you to run with the rescan option set to true.

[morcos]

So you must either provide all timestamps or provide rescan = true. And a left off timestamp = genesis. The sounds reasonable to me , but then I think we need the shorthand for now?

[sipa]

I think it would be safer if the timestamp defaults to the beginning of time. I think it's a better situation to encourage people to use correct timestamps (and rescan=true), rather than have all keys default to 'now'.

[TheBlueMatt]

@gmaxwell that sounds good.
So...

rescan: false - no rescan
no rescan option set - rescan starts at the minimum of the provided timestamps, but if one is missing throw an exception
rescan: true - rescan starts at the minimum of the provided timestamps, or genesis if one or more is missing.

[sipa]

@TheBlueMatt I believe the default for rescan should be true, and there should be no difference between not setting it and passing true.

Rationale: rescanning using -rescan on the command line would result in unexpected behaviour otherwise. The only reasonable condition under which -rescan should skip a block is when all key birthdates are known to be later in time.

[sipa]

Perhaps introducing a special value for 'timestamp' to explicitly indicate a key is new is a possibility (perhaps "now" or -1)?

[TheBlueMatt]

@sipa hmm? How would -rescan result in strange behavior if you threw, instead of doing any importing? Because it failed to do anything -rescan will only look at the things that were previously there, which is unchanged behavior.

[sipa]

I think the simplest change for now is just making the timestamp mandatory. That requirement can later be relaxed in future versions if we agree on sensible defaults.

[luke-jr]

importmulti can import private keys, which is itself a risk to funds loss for end users who don't know what they're doing. So IMO this isn't a blocker.

But not having a default allows for setting a default later, so if we're unsure of what defaults ought to be, simply requiring the user to specify it seems like a good alternative.

[jonasschnelli]

[20:10:05] <wumpus> or have no default at all and require a time to be specified

I think we should make the field mandatory. IMO we can expect API are capable of providing a reasonable timestamp. If they set it to 0, so be it.