Find htlc by id method

[akumaigorodski]

Another minor change to move a somewhat tricky HTLC search logic to a separate method.

[t-bast]

You inverted the place where we look in the remote commit and the one where we look in the local one.
That deserves a closer look, I'd like @pm47 to verify that this isn't an issue (maybe we're missing a test if it is).

[pm47]

remote signs our local commitment, so yes , that's a regression. Agree that this probably means we are missing a non-reg test on this method.

[akumaigorodski]

regression in a sense that a name is wrong? It was remoteSigned <- commitments.localCommit ... and now it's localSigned <- commitments.localCommit ..., this is something I've missed indeed. An order of checks is also changed but is it important?

[pm47]

It'a regression only in the sense that the naming was accurate and it's not anymore. I thought there could be something more, but upon deeper inspection it doesn't appear to be the case.

[pm47]

To remove ambiguity we could rewrite to:

  def getHtlcCrossSigned(commitments: Commitments, directionRelativeToLocal: Direction, htlcId: Long): Option[UpdateAddHtlc] = for {
    localSigned <- commitments.remoteNextCommitInfo.left.toOption.map(_.nextRemoteCommit).getOrElse(commitments.remoteCommit).spec.findHtlcById(htlcId, directionRelativeToLocal.opposite)
    remoteSigned <- commitments.localCommit.spec.findHtlcById(htlcId, directionRelativeToLocal)
  } yield {
    require(localSigned.add == remoteSigned.add)
    localSigned.add
  }

[t-bast]

LGTM, thanks!