commit 3beb830dccaebfcd243a38189187341717982289
parent b8ce66e205120a75884c8649b924ecc2df05d8b7
Author: Jared Tobin <jared@jtobin.io>
Date: Sat, 10 Oct 2026 13:14:01 +0400
Merge fix/validation: remote dependencies, bit wrap, opposite bit
Fixes three validation bugs:
- validateRemote now checks that all known transitive dependencies of
set features are present, as BOLT 1 (init) and BOLT 11 (invoices)
require of the receiver.
- Bit positions derived from a vector (setBits, highestSetBit, and the
bit fields of UnknownRequiredBit and InvalidParity) are now Int
rather than Word16. A vector can be 65535 bytes long; bits at 65536
and above previously wrapped onto low bits, so an unknown even bit
could alias a known feature and pass remote validation.
- setFeature clears the other bit of the feature's pair, so the
constructors (setFeature, setFeatureWithDeps, setFeatureForContext)
can no longer produce a vector with both bits set.
setFeatureWithDeps leaves already-set dependencies at their level.
Adds regression tests for each.
Diffstat:
3 files changed, 151 insertions(+), 23 deletions(-)
diff --git a/lib/Lightning/Protocol/BOLT9/Codec.hs b/lib/Lightning/Protocol/BOLT9/Codec.hs
@@ -89,7 +89,8 @@ testBit !idx = member (bitIndex idx)
-- | Set a feature's bit at the given level.
--
--- 'Required' sets the even bit, 'Optional' sets the odd bit.
+-- 'Required' sets the even bit, 'Optional' sets the odd bit. The
+-- other bit of the pair is cleared, so both are never set.
--
-- >>> import Data.Maybe (fromJust)
-- >>> let mpp = fromJust (featureByName "basic_mpp")
@@ -98,17 +99,17 @@ testBit !idx = member (bitIndex idx)
-- >>> setFeature mpp Required empty -- set required bit (16)
-- FeatureVector {unFeatureVector = "\SOH"}
setFeature :: Feature -> FeatureLevel -> FeatureVector -> FeatureVector
-setFeature !f !level = setBit targetBit
+setFeature !f !level = case level of
+ Required -> setBit baseBit . clearBit (baseBit + 1)
+ Optional -> setBit (baseBit + 1) . clearBit baseBit
where
- !baseBit = featureBaseBit f
- !targetBit = case level of
- Required -> baseBit
- Optional -> baseBit + 1
+ !baseBit = featureBaseBit f
{-# INLINE setFeature #-}
-- | Set a feature and all its transitive dependencies.
--
--- Dependencies are set at the same level as the feature itself.
+-- Dependencies that are not yet set are set at the same level as
+-- the feature itself; dependencies already set keep their level.
-- Unknown dependencies (not in the known features table) are
-- silently skipped.
--
@@ -128,7 +129,11 @@ setFeatureWithDeps !f !level !fv =
where
setDep !depName !acc = case featureByName depName of
Nothing -> acc
- Just dep -> setFeatureWithDeps dep level acc
+ Just dep
+ | isFeatureSet dep acc ->
+ foldr setDep acc (featureDependencies dep)
+ | otherwise ->
+ setFeatureWithDeps dep level acc
-- | Check if a feature is set in the vector.
--
diff --git a/lib/Lightning/Protocol/BOLT9/Validate.hs b/lib/Lightning/Protocol/BOLT9/Validate.hs
@@ -51,10 +51,10 @@ data ValidationError
| ContextNotAllowed !String !Context
-- ^ A feature is not allowed in the given context.
-- Arguments: feature name, context.
- | UnknownRequiredBit {-# UNPACK #-} !Word16
+ | UnknownRequiredBit {-# UNPACK #-} !Int
-- ^ An unknown required (even) bit is set (remote validation only).
-- Argument: bit index.
- | InvalidParity {-# UNPACK #-} !Word16 !Context
+ | InvalidParity {-# UNPACK #-} !Int !Context
-- ^ A bit has invalid parity for a channel context.
-- Arguments: bit index, context (ChanAnnOdd or ChanAnnEven).
deriving (Eq, Show, Generic)
@@ -180,9 +180,9 @@ setFeatureForContext !ctx !f !level !fv
| otherwise
= case channelParity ctx of
Just True | level == Optional ->
- Left (InvalidParity targetBit ctx)
+ Left (InvalidParity (fromIntegral targetBit) ctx)
Just False | level == Required ->
- Left (InvalidParity targetBit ctx)
+ Left (InvalidParity (fromIntegral targetBit) ctx)
_ -> Right (setFeature f level fv)
where
!contexts = featureContexts f
@@ -220,6 +220,7 @@ validateNoBothBits !fv = go knownFeatures
-- * Unknown even (required) bits are errors
-- * If both bits of a pair are set, treat as required (not an error)
-- * Context restrictions still apply for known features
+-- * All dependencies of set features are also set
--
-- >>> import Lightning.Protocol.BOLT9.Codec (setBit)
-- >>> validateRemote Init (setBit 999 empty) -- unknown odd bit: ok
@@ -230,6 +231,7 @@ validateRemote :: Context -> FeatureVector -> Either [ValidationError] ()
validateRemote !ctx !fv =
let errs = unknownRequiredErrors fv
++ contextErrors ctx fv
+ ++ dependencyErrors fv
++ parityErrors ctx fv
in if null errs
then Right ()
@@ -241,7 +243,9 @@ unknownRequiredErrors !fv = foldr check [] (setBits fv)
where
check !bit !acc
| bit `mod` 2 == 1 = acc -- odd bit, optional, ignore
- | otherwise = case featureByBit bit of
+ | bit > fromIntegral (maxBound :: Word16) =
+ UnknownRequiredBit bit : acc
+ | otherwise = case featureByBit (fromIntegral bit) of
Just _ -> acc -- known feature
Nothing -> UnknownRequiredBit bit : acc
@@ -250,7 +254,7 @@ unknownRequiredErrors !fv = foldr check [] (setBits fv)
-- | Find the highest set bit in a feature vector.
--
-- Returns 'Nothing' if the vector is empty or has no bits set.
-highestSetBit :: FeatureVector -> Maybe Word16
+highestSetBit :: FeatureVector -> Maybe Int
highestSetBit !fv =
let !bs = unFeatureVector fv
in if BS.null bs
@@ -258,7 +262,7 @@ highestSetBit !fv =
else findHighestBit bs
-- | Find the highest set bit in a non-empty ByteString.
-findHighestBit :: ByteString -> Maybe Word16
+findHighestBit :: ByteString -> Maybe Int
findHighestBit !bs = go 0
where
!len = BS.length bs
@@ -272,33 +276,33 @@ findHighestBit !bs = go 0
else
let !bytePos = len - 1 - i
!highBit = 7 - B.countLeadingZeros byte
- !bitIdx = fromIntegral bytePos * 8 + fromIntegral highBit
+ !bitIdx = bytePos * 8 + highBit
in Just bitIdx
-- | Collect all set bits in a feature vector.
--
-- Returns a list of bit indices in ascending order.
-setBits :: FeatureVector -> [Word16]
+setBits :: FeatureVector -> [Int]
setBits !fv =
let !bs = unFeatureVector fv
!len = BS.length bs
in collectBits bs len 0 []
-- | Collect bits from a ByteString into a list.
-collectBits :: ByteString -> Int -> Int -> [Word16] -> [Word16]
+collectBits :: ByteString -> Int -> Int -> [Int] -> [Int]
collectBits !bs !len !i !acc
| i >= len = acc
| otherwise =
let !byte = BS.index bs (len - 1 - i)
- !baseIdx = fromIntegral i * 8
+ !baseIdx = i * 8
!acc' = collectByteBits byte baseIdx acc
in collectBits bs len (i + 1) acc'
-- | Collect set bits from a single byte.
-collectByteBits :: B.Bits a => a -> Word16 -> [Word16] -> [Word16]
+collectByteBits :: B.Bits a => a -> Int -> [Int] -> [Int]
collectByteBits !byte !baseIdx = go 7
where
go !bit !acc
| bit < 0 = acc
- | B.testBit byte bit = go (bit - 1) ((baseIdx + fromIntegral bit) : acc)
+ | B.testBit byte bit = go (bit - 1) ((baseIdx + bit) : acc)
| otherwise = go (bit - 1) acc
diff --git a/test/Main.hs b/test/Main.hs
@@ -6,7 +6,7 @@ module Main where
import qualified Data.ByteString as BS
import Data.Either (isLeft, isRight)
import Data.Maybe (isJust, isNothing)
-import Data.Word (Word16)
+import Data.Word (Word8, Word16)
import Lightning.Protocol.BOLT9
import Test.Tasty
import Test.Tasty.HUnit
@@ -25,6 +25,9 @@ tests = testGroup "ppad-bolt9" [
, setFeatureWithDepsTests
, setFeatureForContextTests
, validateNoBothBitsTests
+ , remoteDependencyTests
+ , longVectorTests
+ , oppositeBitTests
, propertyTests
]
@@ -417,6 +420,121 @@ validateNoBothBitsTests = testGroup "validateNoBothBits" [
isLeft (validateNoBothBits fv) @?= True
]
+-- Remote dependency tests -----------------------------------------------------
+
+remoteDependencyTests :: TestTree
+remoteDependencyTests = testGroup "validateRemote dependencies" [
+ testCase "rejects option_zeroconf without option_scid_alias" $
+ validateRemote Init (setBit 51 empty) @?=
+ Left [MissingDependency "option_zeroconf" "option_scid_alias"]
+
+ , testCase "rejects basic_mpp without payment_secret (init)" $
+ validateRemote Init (setBit 17 empty) @?=
+ Left [MissingDependency "basic_mpp" "payment_secret"]
+
+ , testCase "rejects basic_mpp without payment_secret (invoice)" $
+ validateRemote Invoice (setBit 17 empty) @?=
+ Left [MissingDependency "basic_mpp" "payment_secret"]
+
+ , testCase "rejects option_simple_close without anysegwit" $
+ validateRemote Init (setBit 60 empty) @?=
+ Left [ MissingDependency
+ "option_simple_close" "option_shutdown_anysegwit" ]
+
+ , testCase "accepts features with dependencies set" $ do
+ validateRemote Init (setBit 51 (setBit 47 empty)) @?= Right ()
+ validateRemote Invoice (setBit 17 (setBit 14 empty)) @?= Right ()
+ ]
+
+-- Long vector tests -----------------------------------------------------------
+
+-- | A vector of (n + 1) bytes whose most significant byte is b.
+longVector :: Int -> Word8 -> FeatureVector
+longVector n b = parse (BS.cons b (BS.replicate n 0))
+
+longVectorTests :: TestTree
+longVectorTests = testGroup "Long vectors" [
+ testCase "bit 65536 is reported without wrapping" $ do
+ let fv = longVector 8192 0x01
+ setBits fv @?= [65536]
+ highestSetBit fv @?= Just 65536
+ validateRemote Init fv @?= Left [UnknownRequiredBit 65536]
+
+ , testCase "bit 65538 is reported as itself" $
+ validateRemote Init (longVector 8192 0x04) @?=
+ Left [UnknownRequiredBit 65538]
+
+ , testCase "bit 65552 does not alias basic_mpp" $
+ validateRemote Init (longVector 8194 0x01) @?=
+ Left [UnknownRequiredBit 65552]
+
+ , testCase "max-length vector, top odd bit accepted" $ do
+ let fv = longVector 65534 0x80
+ setBits fv @?= [524279]
+ highestSetBit fv @?= Just 524279
+ validateRemote Init fv @?= Right ()
+
+ , testCase "max-length vector, top even bit rejected" $
+ validateRemote Init (longVector 65534 0x40) @?=
+ Left [UnknownRequiredBit 524278]
+ ]
+
+-- Opposite bit tests ----------------------------------------------------------
+
+-- | Look up a known feature by name, failing the test if absent.
+knownFeature :: String -> IO Feature
+knownFeature n = case featureByName n of
+ Nothing -> assertFailure ("feature not found: " ++ n)
+ Just f -> pure f
+
+oppositeBitTests :: TestTree
+oppositeBitTests = testGroup "Opposite bit" [
+ testCase "setFeature Optional clears the required bit" $ do
+ ps <- knownFeature "payment_secret"
+ let fv = setFeature ps Optional (setFeature ps Required empty)
+ setBits fv @?= [15]
+ hasFeature ps fv @?= Just Optional
+
+ , testCase "setFeature Required clears the optional bit" $ do
+ ps <- knownFeature "payment_secret"
+ let fv = setFeature ps Required (setBit 15 empty)
+ setBits fv @?= [14]
+ hasFeature ps fv @?= Just Required
+
+ , testCase "setFeatureWithDeps keeps a set dependency's level" $ do
+ ps <- knownFeature "payment_secret"
+ mpp <- knownFeature "basic_mpp"
+ let fv = setFeatureWithDeps mpp Optional
+ (setFeature ps Required empty)
+ hasFeature ps fv @?= Just Required
+ hasFeature mpp fv @?= Just Optional
+ validateLocal Init fv @?= Right ()
+
+ , testCase "setFeatureForContext clears the opposite bit" $ do
+ anc <- knownFeature "option_anchors"
+ let start = setFeature anc Required empty
+ case setFeatureForContext Init anc Optional start of
+ Left e -> assertFailure (show e)
+ Right fv -> do
+ hasFeature anc fv @?= Just Optional
+ validateNoBothBits fv @?= Right fv
+
+ , testProperty "setFeature never sets both bits (any start)" $
+ forAll genFeatureAndLevel $ \(f, lvl) bs ->
+ let fv = setFeature f lvl (parse (BS.pack bs))
+ b = featureBaseBit f
+ in not (testBit b fv && testBit (b + 1) fv)
+ && hasFeature f fv == Just lvl
+
+ , testProperty "setFeature leaves other bits alone" $
+ forAll genFeatureAndLevel $ \(f, lvl) bs ->
+ let fv0 = parse (BS.pack bs)
+ fv = setFeature f lvl fv0
+ b = fromIntegral (featureBaseBit f)
+ other = filter (\i -> i /= b && i /= b + 1)
+ in other (setBits fv) === other (setBits fv0)
+ ]
+
isInvalidParity :: ValidationError -> Bool
isInvalidParity (InvalidParity _ _) = True
isInvalidParity _ = False
@@ -461,7 +579,8 @@ propertyTests = testGroup "Properties" [
, testProperty "member consistent with setBits" $
\bs -> let fv = parse (BS.pack bs)
bits = setBits fv
- in all (\b -> member (bitIndex b) fv) bits === True
+ in all (\b -> member (bitIndex (fromIntegral b)) fv) bits
+ === True
, testProperty "requiredBit only succeeds for even" $
\(n :: Word16) -> isJust (requiredBit n) === (n `mod` 2 == 0)
