Skip to content

Commit

Permalink
(Minor) Nits (#2139)
Browse files Browse the repository at this point in the history
* consistency in feature names

Since we're not always following the `option_` prefix from the spec,
let's at least be consistent with ourselves.

* add alternative to `def nodeFees()`
  • Loading branch information
pm47 committed Jan 21, 2022
1 parent e2b1b26 commit 2f07b3e
Show file tree
Hide file tree
Showing 19 changed files with 66 additions and 61 deletions.
8 changes: 4 additions & 4 deletions eclair-core/src/main/scala/fr/acinq/eclair/Features.scala
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ object Features {
}
}

case object OptionDataLossProtect extends Feature with InitFeature with NodeFeature {
case object DataLossProtect extends Feature with InitFeature with NodeFeature {
val rfcName = "option_data_loss_protect"
val mandatory = 0
}
Expand All @@ -166,7 +166,7 @@ object Features {
val mandatory = 2
}

case object OptionUpfrontShutdownScript extends Feature with InitFeature with NodeFeature {
case object UpfrontShutdownScript extends Feature with InitFeature with NodeFeature {
val rfcName = "option_upfront_shutdown_script"
val mandatory = 4
}
Expand Down Expand Up @@ -250,9 +250,9 @@ object Features {
}

val knownFeatures: Set[Feature] = Set(
OptionDataLossProtect,
DataLossProtect,
InitialRoutingSync,
OptionUpfrontShutdownScript,
UpfrontShutdownScript,
ChannelRangeQueries,
VariableLengthOnion,
ChannelRangeQueriesExtended,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder, remo
val channelKeyPath = keyManager.keyPath(localParams, channelConfig)
// In order to allow TLV extensions and keep backwards-compatibility, we include an empty upfront_shutdown_script if this feature is not used
// See https://github.com/lightningnetwork/lightning-rfc/pull/714.
val localShutdownScript = if (Features.canUseFeature(localParams.initFeatures, remoteInit.features, Features.OptionUpfrontShutdownScript)) localParams.defaultFinalScriptPubKey else ByteVector.empty
val localShutdownScript = if (Features.canUseFeature(localParams.initFeatures, remoteInit.features, Features.UpfrontShutdownScript)) localParams.defaultFinalScriptPubKey else ByteVector.empty
val open = OpenChannel(nodeParams.chainHash,
temporaryChannelId = temporaryChannelId,
fundingSatoshis = fundingSatoshis,
Expand Down Expand Up @@ -350,7 +350,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder, remo
val minimumDepth = Helpers.minDepthForFunding(nodeParams, open.fundingSatoshis)
// In order to allow TLV extensions and keep backwards-compatibility, we include an empty upfront_shutdown_script if this feature is not used.
// See https://github.com/lightningnetwork/lightning-rfc/pull/714.
val localShutdownScript = if (Features.canUseFeature(localParams.initFeatures, remoteInit.features, Features.OptionUpfrontShutdownScript)) localParams.defaultFinalScriptPubKey else ByteVector.empty
val localShutdownScript = if (Features.canUseFeature(localParams.initFeatures, remoteInit.features, Features.UpfrontShutdownScript)) localParams.defaultFinalScriptPubKey else ByteVector.empty
val accept = AcceptChannel(temporaryChannelId = open.temporaryChannelId,
dustLimitSatoshis = localParams.dustLimit,
maxHtlcValueInFlightMsat = localParams.maxHtlcValueInFlightMsat,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ object ChannelFeatures {
def apply(channelType: ChannelType, localFeatures: Features, remoteFeatures: Features): ChannelFeatures = {
// NB: we don't include features that can be safely activated/deactivated without impacting the channel's operation,
// such as option_dataloss_protect or option_shutdown_anysegwit.
val availableFeatures: Seq[Feature] = Seq(Features.Wumbo, Features.OptionUpfrontShutdownScript).filter(f => Features.canUseFeature(localFeatures, remoteFeatures, f))
val availableFeatures: Seq[Feature] = Seq(Features.Wumbo, Features.UpfrontShutdownScript).filter(f => Features.canUseFeature(localFeatures, remoteFeatures, f))
val allFeatures = channelType.features.toSeq ++ availableFeatures
ChannelFeatures(allFeatures: _*)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ case class Commitments(channelId: ByteVector32,
def getLocalShutdownScript(scriptPubKey: Option[ByteVector]): Either[ChannelException, ByteVector] = {
// to check whether shutdown_any_segwit is active we check features in local and remote parameters, which are negotiated each time we connect to our peer.
val allowAnySegwit = Features.canUseFeature(localParams.initFeatures, remoteParams.initFeatures, Features.ShutdownAnySegwit)
(channelFeatures.hasFeature(Features.OptionUpfrontShutdownScript), scriptPubKey) match {
(channelFeatures.hasFeature(Features.UpfrontShutdownScript), scriptPubKey) match {
case (true, Some(script)) if script != localParams.defaultFinalScriptPubKey => Left(InvalidFinalScript(channelId))
case (false, Some(script)) if !Closing.isValidFinalScriptPubkey(script, allowAnySegwit) => Left(InvalidFinalScript(channelId))
case (false, Some(script)) => Right(script)
Expand All @@ -109,7 +109,7 @@ case class Commitments(channelId: ByteVector32,
def getRemoteShutdownScript(remoteScriptPubKey: ByteVector): Either[ChannelException, ByteVector] = {
// to check whether shutdown_any_segwit is active we check features in local and remote parameters, which are negotiated each time we connect to our peer.
val allowAnySegwit = Features.canUseFeature(localParams.initFeatures, remoteParams.initFeatures, Features.ShutdownAnySegwit)
(channelFeatures.hasFeature(Features.OptionUpfrontShutdownScript), remoteParams.shutdownScript) match {
(channelFeatures.hasFeature(Features.UpfrontShutdownScript), remoteParams.shutdownScript) match {
case (false, _) if !Closing.isValidFinalScriptPubkey(remoteScriptPubKey, allowAnySegwit) => Left(InvalidFinalScript(channelId))
case (false, _) => Right(remoteScriptPubKey)
case (true, None) if !Closing.isValidFinalScriptPubkey(remoteScriptPubKey, allowAnySegwit) =>
Expand Down
10 changes: 5 additions & 5 deletions eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ object Helpers {
}

def extractShutdownScript(channelId: ByteVector32, localFeatures: Features, remoteFeatures: Features, upfrontShutdownScript_opt: Option[ByteVector]): Either[ChannelException, Option[ByteVector]] = {
val canUseUpfrontShutdownScript = Features.canUseFeature(localFeatures, remoteFeatures, Features.OptionUpfrontShutdownScript)
val canUseUpfrontShutdownScript = Features.canUseFeature(localFeatures, remoteFeatures, Features.UpfrontShutdownScript)
val canUseAnySegwit = Features.canUseFeature(localFeatures, remoteFeatures, Features.ShutdownAnySegwit)
extractShutdownScript(channelId, canUseUpfrontShutdownScript, canUseAnySegwit, upfrontShutdownScript_opt)
}
Expand Down Expand Up @@ -516,8 +516,8 @@ object Helpers {
def firstClosingFee(commitments: Commitments, localScriptPubkey: ByteVector, remoteScriptPubkey: ByteVector, feerates: ClosingFeerates)(implicit log: LoggingAdapter): ClosingFees = {
import commitments._
// this is just to estimate the weight, it depends on size of the pubkey scripts
val actualLocalScript = if (channelFeatures.hasFeature(Features.OptionUpfrontShutdownScript)) localParams.defaultFinalScriptPubKey else localScriptPubkey
val actualRemoteScript = if (channelFeatures.hasFeature(Features.OptionUpfrontShutdownScript)) remoteParams.shutdownScript.getOrElse(remoteScriptPubkey) else remoteScriptPubkey
val actualLocalScript = if (channelFeatures.hasFeature(Features.UpfrontShutdownScript)) localParams.defaultFinalScriptPubKey else localScriptPubkey
val actualRemoteScript = if (channelFeatures.hasFeature(Features.UpfrontShutdownScript)) remoteParams.shutdownScript.getOrElse(remoteScriptPubkey) else remoteScriptPubkey
val dummyClosingTx = Transactions.makeClosingTx(commitInput, actualLocalScript, actualRemoteScript, localParams.isFunder, Satoshi(0), Satoshi(0), localCommit.spec)
val closingWeight = Transaction.weight(Transactions.addSigs(dummyClosingTx, dummyPublicKey, remoteParams.fundingPubKey, Transactions.PlaceHolderSig, Transactions.PlaceHolderSig).tx)
log.info(s"using feerates=$feerates for initial closing tx")
Expand Down Expand Up @@ -550,8 +550,8 @@ object Helpers {

def makeClosingTx(keyManager: ChannelKeyManager, commitments: Commitments, localScriptPubkey: ByteVector, remoteScriptPubkey: ByteVector, closingFees: ClosingFees)(implicit log: LoggingAdapter): (ClosingTx, ClosingSigned) = {
import commitments._
val actualLocalScript = if (channelFeatures.hasFeature(Features.OptionUpfrontShutdownScript)) localParams.defaultFinalScriptPubKey else localScriptPubkey
val actualRemoteScript = if (channelFeatures.hasFeature(Features.OptionUpfrontShutdownScript)) remoteParams.shutdownScript.getOrElse(remoteScriptPubkey) else remoteScriptPubkey
val actualLocalScript = if (channelFeatures.hasFeature(Features.UpfrontShutdownScript)) localParams.defaultFinalScriptPubKey else localScriptPubkey
val actualRemoteScript = if (channelFeatures.hasFeature(Features.UpfrontShutdownScript)) remoteParams.shutdownScript.getOrElse(remoteScriptPubkey) else remoteScriptPubkey
val allowAnySegwit = Features.canUseFeature(commitments.localParams.initFeatures, commitments.remoteParams.initFeatures, Features.ShutdownAnySegwit)
require(isValidFinalScriptPubkey(actualLocalScript, allowAnySegwit), "invalid localScriptPubkey")
require(isValidFinalScriptPubkey(actualRemoteScript, allowAnySegwit), "invalid remoteScriptPubkey")
Expand Down
6 changes: 6 additions & 0 deletions eclair-core/src/main/scala/fr/acinq/eclair/package.scala
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ package fr.acinq
import fr.acinq.bitcoin.Crypto.PrivateKey
import fr.acinq.bitcoin._
import fr.acinq.eclair.crypto.StrongRandom
import fr.acinq.eclair.payment.relay.Relayer.RelayFees
import fr.acinq.eclair.wire.protocol.ChannelUpdate
import scodec.Attempt
import scodec.bits.{BitVector, ByteVector}

Expand Down Expand Up @@ -70,6 +72,10 @@ package object eclair {
*/
def nodeFee(baseFee: MilliSatoshi, proportionalFee: Long, paymentAmount: MilliSatoshi): MilliSatoshi = baseFee + (paymentAmount * proportionalFee) / 1000000

def nodeFee(relayFees: RelayFees, paymentAmount: MilliSatoshi): MilliSatoshi = nodeFee(relayFees.feeBase, relayFees.feeProportionalMillionths, paymentAmount)

def nodeFee(channelUpdate: ChannelUpdate, paymentAmount: MilliSatoshi): MilliSatoshi = nodeFee(channelUpdate.feeBaseMsat, channelUpdate.feeProportionalMillionths, paymentAmount)

/**
* @param address base58 of bech32 address
* @param chainHash hash of the chain we're on, which will be checked against the input address
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@

package fr.acinq.eclair.payment.relay

import java.util.UUID

import akka.actor.ActorRef
import akka.actor.typed.Behavior
import akka.actor.typed.eventstream.EventStream
Expand All @@ -29,10 +27,11 @@ import fr.acinq.eclair.db.PendingCommandsDb
import fr.acinq.eclair.payment.Monitoring.{Metrics, Tags}
import fr.acinq.eclair.payment.relay.Relayer.OutgoingChannel
import fr.acinq.eclair.payment.{ChannelPaymentRelayed, IncomingPaymentPacket}
import fr.acinq.eclair.router.Announcements
import fr.acinq.eclair.wire.protocol._
import fr.acinq.eclair.{Logs, NodeParams, ShortChannelId, channel, nodeFee}

import java.util.UUID

object ChannelRelay {

// @formatter:off
Expand Down Expand Up @@ -262,7 +261,7 @@ class ChannelRelay private(nodeParams: NodeParams,
RelayFailure(CMD_FAIL_HTLC(add.id, Right(AmountBelowMinimum(payload.amountToForward, channelUpdate)), commit = true))
case Some(channelUpdate) if r.expiryDelta < channelUpdate.cltvExpiryDelta =>
RelayFailure(CMD_FAIL_HTLC(add.id, Right(IncorrectCltvExpiry(payload.outgoingCltv, channelUpdate)), commit = true))
case Some(channelUpdate) if r.relayFeeMsat < nodeFee(channelUpdate.feeBaseMsat, channelUpdate.feeProportionalMillionths, payload.amountToForward) =>
case Some(channelUpdate) if r.relayFeeMsat < nodeFee(channelUpdate, payload.amountToForward) =>
RelayFailure(CMD_FAIL_HTLC(add.id, Right(FeeInsufficient(add.amountMsat, channelUpdate)), commit = true))
case Some(channelUpdate) =>
val origin = Origin.ChannelRelayedHot(addResponseAdapter.toClassic, add, payload.amountToForward)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ object NodeRelay {
}

def validateRelay(nodeParams: NodeParams, upstream: Upstream.Trampoline, payloadOut: PaymentOnion.NodeRelayPayload): Option[FailureMessage] = {
val fee = nodeFee(nodeParams.relayParams.minTrampolineFees.feeBase, nodeParams.relayParams.minTrampolineFees.feeProportionalMillionths, payloadOut.amountToForward)
val fee = nodeFee(nodeParams.relayParams.minTrampolineFees, payloadOut.amountToForward)
if (upstream.amountIn - payloadOut.amountToForward < fee) {
Some(TrampolineFeeInsufficient)
} else if (upstream.expiryIn - payloadOut.outgoingCltv < nodeParams.expiryDelta) {
Expand Down Expand Up @@ -134,7 +134,7 @@ object NodeRelay {
*/
def translateError(nodeParams: NodeParams, failures: Seq[PaymentFailure], upstream: Upstream.Trampoline, nextPayload: PaymentOnion.NodeRelayPayload): Option[FailureMessage] = {
val routeNotFound = failures.collectFirst { case f@LocalFailure(_, _, RouteNotFound) => f }.nonEmpty
val routingFeeHigh = upstream.amountIn - nextPayload.amountToForward >= nodeFee(nodeParams.relayParams.minTrampolineFees.feeBase, nodeParams.relayParams.minTrampolineFees.feeProportionalMillionths, nextPayload.amountToForward) * 5
val routingFeeHigh = upstream.amountIn - nextPayload.amountToForward >= nodeFee(nodeParams.relayParams.minTrampolineFees, nextPayload.amountToForward) * 5
failures match {
case Nil => None
case LocalFailure(_, _, BalanceTooLow) :: Nil if routingFeeHigh =>
Expand Down
10 changes: 5 additions & 5 deletions eclair-core/src/main/scala/fr/acinq/eclair/router/Graph.scala
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ object Graph {
val totalCltv = prev.cltv + cltv
weightRatios match {
case Left(weightRatios) =>
val hopCost = nodeFee(weightRatios.hopCost.feeBase, weightRatios.hopCost.feeProportionalMillionths, prev.amount)
val hopCost = nodeFee(weightRatios.hopCost, prev.amount)
import RoutingHeuristics._

// Every edge is weighted by funding block height where older blocks add less weight. The window considered is 1 year.
Expand All @@ -322,7 +322,7 @@ object Graph {
val totalWeight = prev.weight + (fee + hopCost).toLong * factor
RichWeight(totalAmount, prev.length + 1, totalCltv, 1.0, totalFees, 0 msat, totalWeight)
case Right(heuristicsConstants) =>
val hopCost = nodeFee(heuristicsConstants.hopCost.feeBase, heuristicsConstants.hopCost.feeProportionalMillionths, prev.amount)
val hopCost = nodeFee(heuristicsConstants.hopCost, prev.amount)
val totalHopsCost = prev.virtualFees + hopCost
val riskCost = totalAmount.toLong * totalCltv.toInt * heuristicsConstants.lockedFundsRisk
// If the edge was added by the invoice, it is assumed that it can route the payment.
Expand All @@ -332,7 +332,7 @@ object Graph {
throw NegativeProbability(edge, prev, heuristicsConstants)
}
val totalSuccessProbability = prev.successProbability * successProbability
val failureCost = nodeFee(heuristicsConstants.failureCost.feeBase, heuristicsConstants.failureCost.feeProportionalMillionths, totalAmount)
val failureCost = nodeFee(heuristicsConstants.failureCost, totalAmount)
val weight = totalFees.toLong + totalHopsCost.toLong + riskCost + failureCost.toLong / totalSuccessProbability
RichWeight(totalAmount, prev.length + 1, totalCltv, totalSuccessProbability, totalFees, totalHopsCost, weight)
}
Expand All @@ -347,7 +347,7 @@ object Graph {
* @return the new amount updated with the necessary fees for this edge
*/
private def addEdgeFees(edge: GraphEdge, amountToForward: MilliSatoshi): MilliSatoshi = {
amountToForward + nodeFee(edge.update.feeBaseMsat, edge.update.feeProportionalMillionths, amountToForward)
amountToForward + nodeFee(edge.update, amountToForward)
}

/** Validate that all edges along the path can relay the amount with fees. */
Expand Down Expand Up @@ -424,7 +424,7 @@ object Graph {
Some(capacity.toMilliSatoshi - reservedCapacity)
).flatten.min.max(0 msat)

def fee(amount: MilliSatoshi): MilliSatoshi = nodeFee(update.feeBaseMsat, update.feeProportionalMillionths, amount)
def fee(amount: MilliSatoshi): MilliSatoshi = nodeFee(update, amount)

}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ object Router {
case class ChannelHop(nodeId: PublicKey, nextNodeId: PublicKey, lastUpdate: ChannelUpdate) extends Hop {
override lazy val cltvExpiryDelta: CltvExpiryDelta = lastUpdate.cltvExpiryDelta

override def fee(amount: MilliSatoshi): MilliSatoshi = nodeFee(lastUpdate.feeBaseMsat, lastUpdate.feeProportionalMillionths, amount)
override def fee(amount: MilliSatoshi): MilliSatoshi = nodeFee(lastUpdate, amount)
}

/**
Expand Down

0 comments on commit 2f07b3e

Please sign in to comment.