Replace ErrNotFound with boolean return value

[anorth]

Removes ErrNotFound and replaces its use with an additional boolean return value from Get, Delete, BatchDelete and Subtract.

Depending on your view of whether ErrNotFound is a true error, this changes the semantics of Delete to make it not an "error" to attempt to delete a missing key.

This definitely changes the semantics of, BatchDelete and Subtract, which can now successfully delete only some of the keys requested. Subtract is unused in Filecoin actors, but BatchDelete is used quite a bit in the expiration and partition queues (rescheduling expirations, popping as queues). I've picked an easy (for the AMT) option of allowing keys to not exist. This would reduce the implicit consistency checking that the prior BatchDelete semantics offered. Callers would need to explicitly do something compare expected Count to check if all expected keys were actually deleted.

We have a few options here, and I'm keen for discussion

• Return boolean modified, true if any keys were found/removed (currently implemented)
• Return true if all requested keys were found and removed
• Return a count of keys found and removed
• Return a slice of the keys found and removed
• Add a parameter indicating whether to expect all keys to exist, either considering a missing key an error, or changing the semantics of the boolean return value (any vs all).

[rvagg]

If Subtract isn't used by spec-actors maybe it needs to be removed?

Re BatchDelete options - that really depends on how much variety there is in the ways it's used? Is there a consistent pattern that can have some consistency assertions built around it? Do we know, in each instance it's called, that all of the keys should be there? Or are there times when we're just shooting fish in a barrel? It'd not be great to reduce the options for consistency checking by allowing unexpected state to be silently accepted when we have the opportunity to check for it.

[Stebalien]

For batch delete, I'd either return a boolean for "all keys found", or a count. The new API is quite a bit safer for us, actually. IIRC, we've been sloppy in the past and just assumed that everything existsd.

[anorth]

All the existing cases do expect all keys to be present (b/c of the previous semantics). @rvagg's point about not reducing the ease of that case is strong.

Based on that, I will:

• Add a new bool parameter indicating whether to expect all keys to be found, and return an error if they are not
• Have the return value true if any key is found and the structure modified

This results in a very simple change to existing call sites, which can pass true and ignore the return value. When the (hypothetical future) caller is not sure whether all the keys exist or not, indicating whether any existed rather than all is more likely to be helpful. This follows general Set APIs in other languages/frameworks, none of which I could find returning a count.

[Kubuxu]

I'm assuming this goes into v3. instead of creating v4, as the only user of v3 so far was actors version v3.

[anorth]

@Kubuxu Yes this is all part of v3 development on master. It's not considered stable until I tag a release (right after this PR).