Skip to content

Flat features #1253

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
t-bast merged 9 commits into from
Jan 9, 2020
Merged

Flat features #1253

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
a6da1db
Flat Features
t-bast Dec 12, 2019
2a50f17
Remove allowMultiPart API field
t-bast Dec 12, 2019
97adf0b
Rework features
t-bast Dec 17, 2019
120f773
Use new feature types in Trampoline code
t-bast Dec 18, 2019
9c595e3
Combine feature in codec, not type
t-bast Dec 19, 2019
1b2daaa
Fix an unrelated flaky test
t-bast Dec 19, 2019
ea7abd9
Refactoring from PR comments
t-bast Jan 9, 2020
9330c0b
Merge branch 'master' into flat-features
t-bast Jan 9, 2020
9bd3059
Remove redundant default argument of None for hasFeature
t-bast Jan 9, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 6 additions & 8 deletions eclair-core/src/main/resources/reference.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,12 @@ eclair {
node-color = "49daaa"

trampoline-payments-enable = false // TODO: @t-bast: once spec-ed this should use a global feature flag
Comment thread
t-bast marked this conversation as resolved.
global-features = "0200" // variable_length_onion
local-features = "088a" // initial_routing_sync + option_data_loss_protect + option_channel_range_queries + option_channel_range_queries_ex
features = "0a8a" // initial_routing_sync + option_data_loss_protect + option_channel_range_queries + option_channel_range_queries_ex + variable_length_onion
Comment thread
t-bast marked this conversation as resolved.
Copy link
Copy Markdown
Member

@pm47 pm47 Dec 30, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should use a completely different method, using rfcName field:

features {
  option_data_loss_protect = "mandatory"
  initial_routing_sync = "optional"
  gossip_queries = "mandatory"
  ...
}

Less error prone and more readable, but also less flexible.

Copy link
Copy Markdown
Member

@pm47 pm47 Dec 30, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would also affect the override-features attribute...

Copy link
Copy Markdown
Member Author

@t-bast t-bast Dec 30, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's a good idea if we really want to allow users to turn some features on and off.
The problem is there are quite a few features that we can't really turn off right now (some code paths will ignore the fact that a feature is turned off and still use it).
I think it makes sense to do that change once we've worked on making sure the code base really allows completely turning off all features.

override-features = [ // optional per-node features
# {
# nodeid = "02aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
# global-features = "",
# local-features = ""
# }
# {
# nodeid = "02aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
# features = ""
# }
]
sync-whitelist = [] // a list of public keys; if non-empty, we will only do the initial sync with those peers
channel-flags = 1 // announce channels
Expand Down Expand Up @@ -132,7 +130,7 @@ eclair {
// the values below will be used to perform route searching
path-finding {
max-route-length = 6 // max route length for the 'first pass', if none is found then a second pass is made with no limit
max-cltv = 1008 // max acceptable cltv expiry for the payment (1008 ~ 1 week)
max-cltv = 1008 // max acceptable cltv expiry for the payment (1008 ~ 1 week)
fee-threshold-sat = 21 // if fee is below this value we skip the max-fee-pct check
max-fee-pct = 0.03 // route will be discarded if fee is above this value (in percentage relative to the total payment amount); doesn't apply if fee < fee-threshold-sat

Expand Down
14 changes: 7 additions & 7 deletions eclair-core/src/main/scala/fr/acinq/eclair/Eclair.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,11 @@ import fr.acinq.eclair.channel._
import fr.acinq.eclair.db.{IncomingPayment, NetworkFee, OutgoingPayment, Stats}
import fr.acinq.eclair.io.Peer.{GetPeerInfo, PeerInfo}
import fr.acinq.eclair.io.{NodeURI, Peer}
import fr.acinq.eclair.payment.send.PaymentInitiator.{SendPaymentRequest, SendTrampolinePaymentRequest}
import fr.acinq.eclair.payment.relay.Relayer.{GetOutgoingChannels, OutgoingChannels, UsableBalance}
import fr.acinq.eclair.payment._
import fr.acinq.eclair.payment.receive.MultiPartHandler.ReceivePayment
import fr.acinq.eclair.router.{Announcements, ChannelDesc, GetNetworkStats, NetworkStats, PublicChannel, RouteRequest, RouteResponse, Router}
import fr.acinq.eclair.payment.relay.Relayer.{GetOutgoingChannels, OutgoingChannels, UsableBalance}
import fr.acinq.eclair.payment.send.PaymentInitiator.{SendPaymentRequest, SendTrampolinePaymentRequest}
import fr.acinq.eclair.router._
import fr.acinq.eclair.wire.{ChannelAnnouncement, ChannelUpdate, NodeAddress, NodeAnnouncement}
import scodec.bits.ByteVector

Expand Down Expand Up @@ -80,7 +80,7 @@ trait Eclair {

def peersInfo()(implicit timeout: Timeout): Future[Iterable[PeerInfo]]

def receive(description: String, amount_opt: Option[MilliSatoshi], expire_opt: Option[Long], fallbackAddress_opt: Option[String], paymentPreimage_opt: Option[ByteVector32], allowMultiPart: Boolean)(implicit timeout: Timeout): Future[PaymentRequest]
def receive(description: String, amount_opt: Option[MilliSatoshi], expire_opt: Option[Long], fallbackAddress_opt: Option[String], paymentPreimage_opt: Option[ByteVector32])(implicit timeout: Timeout): Future[PaymentRequest]

def newAddress(): Future[String]

Expand Down Expand Up @@ -190,17 +190,17 @@ class EclairImpl(appKit: Kit) extends Eclair {
override def allUpdates(nodeId_opt: Option[PublicKey])(implicit timeout: Timeout): Future[Iterable[ChannelUpdate]] = nodeId_opt match {
case None => (appKit.router ? 'updates).mapTo[Iterable[ChannelUpdate]]
case Some(pk) => (appKit.router ? 'channelsMap).mapTo[Map[ShortChannelId, PublicChannel]].map { channels =>
channels.map(_._2).flatMap {
channels.values.flatMap {
case PublicChannel(ann, _, _, Some(u1), _) if ann.nodeId1 == pk && u1.isNode1 => List(u1)
case PublicChannel(ann, _, _, _, Some(u2)) if ann.nodeId2 == pk && !u2.isNode1 => List(u2)
case PublicChannel(_, _, _, _, _) => List.empty
}
}
}

override def receive(description: String, amount_opt: Option[MilliSatoshi], expire_opt: Option[Long], fallbackAddress_opt: Option[String], paymentPreimage_opt: Option[ByteVector32], allowMultiPart: Boolean)(implicit timeout: Timeout): Future[PaymentRequest] = {
override def receive(description: String, amount_opt: Option[MilliSatoshi], expire_opt: Option[Long], fallbackAddress_opt: Option[String], paymentPreimage_opt: Option[ByteVector32])(implicit timeout: Timeout): Future[PaymentRequest] = {
fallbackAddress_opt.map { fa => fr.acinq.eclair.addressToPublicKeyScript(fa, appKit.nodeParams.chainHash) } // if it's not a bitcoin address throws an exception
(appKit.paymentHandler ? ReceivePayment(amount_opt, description, expire_opt, fallbackAddress = fallbackAddress_opt, paymentPreimage = paymentPreimage_opt, allowMultiPart = allowMultiPart)).mapTo[PaymentRequest]
(appKit.paymentHandler ? ReceivePayment(amount_opt, description, expire_opt, fallbackAddress = fallbackAddress_opt, paymentPreimage = paymentPreimage_opt)).mapTo[PaymentRequest]
}

override def newAddress(): Future[String] = {
Expand Down
138 changes: 97 additions & 41 deletions eclair-core/src/main/scala/fr/acinq/eclair/Features.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,63 +19,119 @@ package fr.acinq.eclair
import scodec.bits.{BitVector, ByteVector}

/**
* Created by PM on 13/02/2017.
*/
* Created by PM on 13/02/2017.
*/

sealed trait FeatureSupport

// @formatter:off
object FeatureSupport {
case object Mandatory extends FeatureSupport
case object Optional extends FeatureSupport
}
// @formatter:on

sealed trait Feature {
Comment thread
pm47 marked this conversation as resolved.
def rfcName: String

def mandatory: Int

def optional: Int = mandatory + 1

override def toString = rfcName
}

object Features {
Comment thread
pm47 marked this conversation as resolved.
val OPTION_DATA_LOSS_PROTECT_MANDATORY = 0
val OPTION_DATA_LOSS_PROTECT_OPTIONAL = 1

// reserved but not used as per lightningnetwork/lightning-rfc/pull/178
//val INITIAL_ROUTING_SYNC_BIT_MANDATORY = 2
val INITIAL_ROUTING_SYNC_BIT_OPTIONAL = 3
case object OptionDataLossProtect extends Feature {
val rfcName = "option_data_loss_protect"
val mandatory = 0
}

val CHANNEL_RANGE_QUERIES_BIT_MANDATORY = 6
val CHANNEL_RANGE_QUERIES_BIT_OPTIONAL = 7
case object InitialRoutingSync extends Feature {
val rfcName = "initial_routing_sync"
// reserved but not used as per lightningnetwork/lightning-rfc/pull/178
val mandatory = 2
}

val VARIABLE_LENGTH_ONION_MANDATORY = 8
val VARIABLE_LENGTH_ONION_OPTIONAL = 9
case object ChannelRangeQueries extends Feature {
val rfcName = "gossip_queries"
val mandatory = 6
}

case object VariableLengthOnion extends Feature {
val rfcName = "var_onion_optin"
val mandatory = 8
}

val CHANNEL_RANGE_QUERIES_EX_BIT_MANDATORY = 10
val CHANNEL_RANGE_QUERIES_EX_BIT_OPTIONAL = 11
case object ChannelRangeQueriesExtended extends Feature {
val rfcName = "gossip_queries_ex"
val mandatory = 10
}

val PAYMENT_SECRET_MANDATORY = 14
val PAYMENT_SECRET_OPTIONAL = 15
case object PaymentSecret extends Feature {
val rfcName = "payment_secret"
val mandatory = 14
}

val BASIC_MULTI_PART_PAYMENT_MANDATORY = 16
val BASIC_MULTI_PART_PAYMENT_OPTIONAL = 17
case object BasicMultiPartPayment extends Feature {
val rfcName = "basic_mpp"
val mandatory = 16
}

// TODO: @t-bast: update feature bits once spec-ed (currently reserved here: https://github.com/lightningnetwork/lightning-rfc/issues/605)
// We're not advertizing these bits yet in our announcements, clients have to assume support.
// We're not advertising these bits yet in our announcements, clients have to assume support.
// This is why we haven't added them yet to `areSupported`.
val TRAMPOLINE_PAYMENT_MANDATORY = 50
val TRAMPOLINE_PAYMENT_OPTIONAL = 51
case object TrampolinePayment extends Feature {
val rfcName = "trampoline_payment"
val mandatory = 50
}

// Features may depend on other features, as specified in Bolt 9.
private val featuresDependency = Map(
Comment thread
t-bast marked this conversation as resolved.
ChannelRangeQueriesExtended -> (ChannelRangeQueries :: Nil),
PaymentSecret -> (VariableLengthOnion :: Nil),
BasicMultiPartPayment -> (PaymentSecret :: Nil),
TrampolinePayment -> (PaymentSecret :: Nil)
)

case class FeatureException(message: String) extends IllegalArgumentException(message)

def validateFeatureGraph(features: BitVector): Option[FeatureException] = featuresDependency.collectFirst {
case (feature, dependencies) if hasFeature(features, feature) && dependencies.exists(d => !hasFeature(features, d)) =>
FeatureException(s"${features.toBin} sets $feature but is missing a dependency (${dependencies.filter(d => !hasFeature(features, d)).mkString(" and ")})")
}

def validateFeatureGraph(features: ByteVector): Option[FeatureException] = validateFeatureGraph(features.bits)

// Note that BitVector indexes from left to right whereas the specification indexes from right to left.
// This is why we have to reverse the bits to check if a feature is set.

def hasFeature(features: BitVector, bit: Int): Boolean = if (features.sizeLessThanOrEqual(bit)) false else features.reverse.get(bit)
private def hasFeature(features: BitVector, bit: Int): Boolean = features.sizeGreaterThan(bit) && features.reverse.get(bit)

def hasFeature(features: ByteVector, bit: Int): Boolean = hasFeature(features.bits, bit)
def hasFeature(features: BitVector, feature: Feature, support: Option[FeatureSupport] = None): Boolean = support match {
case Some(FeatureSupport.Mandatory) => hasFeature(features, feature.mandatory)
case Some(FeatureSupport.Optional) => hasFeature(features, feature.optional)
case None => hasFeature(features, feature.optional) || hasFeature(features, feature.mandatory)
}

/**
* We currently don't distinguish mandatory and optional. Interpreting VARIABLE_LENGTH_ONION_MANDATORY strictly would
* be very restrictive and probably fork us out of the network.
* We may implement this distinction later, but for now both flags are interpreted as an optional support.
*/
def hasVariableLengthOnion(features: ByteVector): Boolean = hasFeature(features, VARIABLE_LENGTH_ONION_MANDATORY) || hasFeature(features, VARIABLE_LENGTH_ONION_OPTIONAL)
def hasFeature(features: ByteVector, feature: Feature): Boolean = hasFeature(features.bits, feature)

def hasFeature(features: ByteVector, feature: Feature, support: Option[FeatureSupport]): Boolean = hasFeature(features.bits, feature, support)

/**
* Check that the features that we understand are correctly specified, and that there are no mandatory features that
* we don't understand (even bits).
*/
* Check that the features that we understand are correctly specified, and that there are no mandatory features that
* we don't understand (even bits).
*/
def areSupported(features: BitVector): Boolean = {
val supportedMandatoryFeatures = Set[Long](
OPTION_DATA_LOSS_PROTECT_MANDATORY,
CHANNEL_RANGE_QUERIES_BIT_MANDATORY,
VARIABLE_LENGTH_ONION_MANDATORY,
CHANNEL_RANGE_QUERIES_EX_BIT_MANDATORY,
PAYMENT_SECRET_MANDATORY,
BASIC_MULTI_PART_PAYMENT_MANDATORY)
val supportedMandatoryFeatures = Set(
OptionDataLossProtect,
ChannelRangeQueries,
VariableLengthOnion,
ChannelRangeQueriesExtended,
PaymentSecret,
BasicMultiPartPayment
).map(_.mandatory.toLong)
val reversed = features.reverse
for (i <- 0L until reversed.length by 2) {
if (reversed.get(i) && !supportedMandatoryFeatures.contains(i)) return false
Expand All @@ -85,9 +141,9 @@ object Features {
}

/**
* A feature set is supported if all even bits are supported.
* We just ignore unknown odd bits.
*/
* A feature set is supported if all even bits are supported.
* We just ignore unknown odd bits.
*/
def areSupported(features: ByteVector): Boolean = areSupported(features.bits)

}
28 changes: 17 additions & 11 deletions eclair-core/src/main/scala/fr/acinq/eclair/NodeParams.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,8 @@ case class NodeParams(keyManager: KeyManager,
alias: String,
color: Color,
publicAddresses: List[NodeAddress],
globalFeatures: ByteVector,
localFeatures: ByteVector,
overrideFeatures: Map[PublicKey, (ByteVector, ByteVector)],
features: ByteVector,
overrideFeatures: Map[PublicKey, ByteVector],
syncWhitelist: Set[PublicKey],
dustLimit: Satoshi,
onChainFeeConf: OnChainFeeConf,
Expand Down Expand Up @@ -84,6 +83,7 @@ case class NodeParams(keyManager: KeyManager,
enableTrampolinePayment: Boolean) {
val privateKey = keyManager.nodeKey.privateKey
val nodeId = keyManager.nodeId

def currentBlockHeight: Long = blockCount.get
}

Expand Down Expand Up @@ -130,11 +130,15 @@ object NodeParams {
}

def makeNodeParams(config: Config, keyManager: KeyManager, torAddress_opt: Option[NodeAddress], database: Databases, blockCount: AtomicLong, feeEstimator: FeeEstimator): NodeParams = {
// check configuration for keys that have been renamed in v0.3.2
// check configuration for keys that have been renamed
val deprecatedKeyPaths = Map(
// v0.3.2
"default-feerates" -> "on-chain-fees.default-feerates",
"max-feerate-mismatch" -> "on-chain-fees.max-feerate-mismatch",
"update-fee_min-diff-ratio" -> "on-chain-fees.update-fee-min-diff-ratio"
"update-fee_min-diff-ratio" -> "on-chain-fees.update-fee-min-diff-ratio",
// v0.3.3
"global-features" -> "features",
"local-features" -> "features"
)
deprecatedKeyPaths.foreach {
case (old, new_) => require(!config.hasPath(old), s"configuration key '$old' has been replaced by '$new_'")
Expand Down Expand Up @@ -170,11 +174,14 @@ object NodeParams {
val nodeAlias = config.getString("node-alias")
require(nodeAlias.getBytes("UTF-8").length <= 32, "invalid alias, too long (max allowed 32 bytes)")

val overrideFeatures: Map[PublicKey, (ByteVector, ByteVector)] = config.getConfigList("override-features").map { e =>
val features = ByteVector.fromValidHex(config.getString("features"))
val featuresErr = Features.validateFeatureGraph(features)
require(featuresErr.isEmpty, featuresErr.map(_.message))

val overrideFeatures: Map[PublicKey, ByteVector] = config.getConfigList("override-features").map { e =>
val p = PublicKey(ByteVector.fromValidHex(e.getString("nodeid")))
val gf = ByteVector.fromValidHex(e.getString("global-features"))
val lf = ByteVector.fromValidHex(e.getString("local-features"))
p -> (gf, lf)
val f = ByteVector.fromValidHex(e.getString("features"))
p -> f
}.toMap

val syncWhitelist: Set[PublicKey] = config.getStringList("sync-whitelist").map(s => PublicKey(ByteVector.fromValidHex(s))).toSet
Expand Down Expand Up @@ -219,8 +226,7 @@ object NodeParams {
alias = nodeAlias,
color = Color(color(0), color(1), color(2)),
publicAddresses = addresses,
globalFeatures = ByteVector.fromValidHex(config.getString("global-features")),
localFeatures = ByteVector.fromValidHex(config.getString("local-features")),
features = features,
overrideFeatures = overrideFeatures,
syncWhitelist = syncWhitelist,
dustLimit = dustLimitSatoshis,
Expand Down
6 changes: 2 additions & 4 deletions eclair-core/src/main/scala/fr/acinq/eclair/channel/Channel.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,8 +313,7 @@ class Channel(val nodeParams: NodeParams, val wallet: EclairWallet, remoteNodeId
paymentBasepoint = open.paymentBasepoint,
delayedPaymentBasepoint = open.delayedPaymentBasepoint,
htlcBasepoint = open.htlcBasepoint,
globalFeatures = remoteInit.globalFeatures,
localFeatures = remoteInit.localFeatures)
features = remoteInit.features)
log.debug(s"remote params: $remoteParams")
goto(WAIT_FOR_FUNDING_CREATED) using DATA_WAIT_FOR_FUNDING_CREATED(open.temporaryChannelId, localParams, remoteParams, open.fundingSatoshis, open.pushMsat, open.feeratePerKw, open.firstPerCommitmentPoint, open.channelFlags, channelVersion, accept) sending accept
}
Expand Down Expand Up @@ -346,8 +345,7 @@ class Channel(val nodeParams: NodeParams, val wallet: EclairWallet, remoteNodeId
paymentBasepoint = accept.paymentBasepoint,
delayedPaymentBasepoint = accept.delayedPaymentBasepoint,
htlcBasepoint = accept.htlcBasepoint,
globalFeatures = remoteInit.globalFeatures,
localFeatures = remoteInit.localFeatures)
features = remoteInit.features)
log.debug(s"remote params: $remoteParams")
val localFundingPubkey = keyManager.fundingPublicKey(localParams.fundingKeyPath)
val fundingPubkeyScript = Script.write(Script.pay2wsh(Scripts.multiSig2of2(localFundingPubkey.publicKey, remoteParams.fundingPubKey)))
Expand Down
6 changes: 2 additions & 4 deletions eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelTypes.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,7 @@ final case class LocalParams(nodeId: PublicKey,
maxAcceptedHtlcs: Int,
isFunder: Boolean,
defaultFinalScriptPubKey: ByteVector,
globalFeatures: ByteVector,
localFeatures: ByteVector)
features: ByteVector)

final case class RemoteParams(nodeId: PublicKey,
dustLimit: Satoshi,
Expand All @@ -227,8 +226,7 @@ final case class RemoteParams(nodeId: PublicKey,
paymentBasepoint: PublicKey,
delayedPaymentBasepoint: PublicKey,
htlcBasepoint: PublicKey,
globalFeatures: ByteVector,
localFeatures: ByteVector)
features: ByteVector)

object ChannelFlags {
val AnnounceChannel = 0x01.toByte
Expand Down
Loading