consensus: Expose ephemerality of elements in Apply/RevertUpdate

[lukechampine]

This adds a created bool argument to the (Update).ForEach methods, allowing callers to identify ephemeral elements by checking for created && spent (or, in the case of file contracts, created && resolved.

This is very useful for any callers who care about persistently tracking unspent outputs. Normally, the code for doing so looks like this:

cau.ForEachSiacoinElement(func(sce types.SiacoinElement, spent bool) {
    if spent {
        deleteUTXO(sce.ID)
    } else {
        insertUTXO(sce)
    }
})

but this is subtly buggy: if the element was ephemeral, spent will be true, so we'll try to delete it; but since it's ephemeral, it's not in the set, so there's nothing to delete! Whether or not this causes problems depends on how deleteUTXO is implemented. Specifically, if you assert that an element must exist in order to be deleted, that assert will fail.

To avoid this, we need a way to detect whether an element is ephemeral. chain.DBStore attempted to do this by checking sce.LeafIndex == types.EphemeralLeafIndex. However, this doesn't work: ephemeral elements are assigned leaf indices just like regular outputs, so that condition is never satisfied! This eventually lead to a panic: ephemeral outputs within a reverted block would be added to the UTXO set (instead of being ignored), and if a future block references that same output, the DB gets confused and crashes.

Adding a created flag seemed like the most direct of doing this. Another option would be to always skip the ephemeral elements in the ForEach methods, and add a new method (or methods) for accessing only the ephemeral elements. This is attractive, since it doesn't break any existing code, and most callers want to skip ephemeral elements anyway. Ultimately, though, I decided against it; it pollutes the namespace, and it's one more thing you "just need to know" in order to use the API properly. Plus it makes the name "ForEach" a lie. Better to keep everything in one place.

[n8maninger]

Since it seems like EphemeralLeafIndex shouldn't really be used can it potentially be moved to the consensus package and unexported?

[lukechampine]

I wondered about that. You do need it when constructing v2 transaction chains, but since we already have the EphemeralSiacoinOutput helper method that sets it for you, perhaps the value itself could be unexported. 🤔

[lukechampine]

Ok, tried that. The results are pretty good, but there are a few places where the caller definitely needs to know whether an element is ephemeral or not. Namely, when deciding which elements to call UpdateElementProof on! I suppose we could make it a no-op for ephemeral elements (instead of panicking), but that makes me a little uneasy. There's also ChainManager.applyPoolUpdate and ChainManager.revertPoolUpdate: if there's an ephemeral element in the txpool, applying a block might cause it to become non-ephemeral, and vice versa. So I think we probably do want to export the constant, even though it's only strictly necessary in a few places.

[lukechampine]

I pushed a workaround to SiaFoundation/coreutils#65, lmk what you think

[n8maninger]

var ephemeralLeafIndex = consensus.UnassignedStateElement(types.Hash256{}).LeafIndex

Hmm.. That's kind of nasty.. I'd probably lean towards making UpdateElementProof a no-op for unassignedLeafIndex. Otherwise, a helper func IsUnassignedElement(se *StateElement) bool or leaving it as is. I don't have a strong preference on any of those.

[lukechampine]

Another option: just ban ephemeral outputs :P
(I feel like there's as least one strong reason not to do that, but I don't remember it off the top of my head...)

I think it's probably best to mostly return to the status quo, with an exported const in types. If we rename it to UnassignedLeafIndex, that'll cause compilation errors for all existing usages, which we can then audit for misuse.