Move RBF-bump to start of update_claims_view to avoid double-bump

[TheBlueMatt]

`OnchainTx::update_claims_view` currently checks if there are
packages which need to be RBF-bumped first, prior to adding new
packages to the tracking map. This can, in some cases, cause us to
RBF-bump a package which we only just added, at least in cases
where we're skipping blocks being connected.

Here we simply move the RBF-bump check to the top of
`OnchainTx::update_claims_view`, avoiding any such cases (though
there are still some cases where we may call
`OnchainTx::update_claims_view` twice for the same block, which is
resolved separately).

This commit doesn't appear to cause any test failures, but should get test coverage to confirm it actually does what it looks like it does, as it may not. https://git.bitcoin.ninja/index.cgi?p=rust-lightning;a=commit;h=7b161f7b0feb0fbe7d6465eb1d705c5b1bddf554

[wpaulino]

I don't think this was ever an issue? The timer on the new request would be greater than the current height after generating its first claim, so it would get filtered out when looking for those which need to be bumped.

[TheBlueMatt]

I'm pretty sure I saw it, which is why I opened this. Its kinda annoying to trace now, but the commit message above notes specifically that this happens in case of block-skipping connections, ie the case where conf_height (the height the request may have in it) is different from cur_height (the height we're using to decide whether to RBF).

[wpaulino]

Wouldn't this only happen when the requests originate from the same best_block_updated call that is causing bumps to be done? For example, let's say we need to go onchain to claim an HTLC at height 100, but we skip it and connect 103 instead. The claim will be using the latest block (height 103 in this case) to evaluate when it should be retried, so it should be filtered out when evaluating claims that need to be bumped?

In the case where the request comes from another source, like a preimage, we'd use the current block to evaluate the retry timer. If the next block we connect ends up skipping some and it meets the retry height, it seems we should be bumping anyway, even if we just generated the claim right before the block gets connected, in order to meet our deadline.