multi: rename experimental endorsement signal to accountable #10367
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -290,9 +290,9 @@ type ChannelLinkConfig struct { | |
| // restrict the flow of HTLCs and fee updates. | ||
| MaxFeeExposure lnwire.MilliSatoshi | ||
|
|
||
| // ShouldFwdExpAccountability is a closure that indicates whether the | ||
| // link should forward experimental accountability signals. | ||
| ShouldFwdExpAccountability func() bool | ||
|
|
||
| // AuxTrafficShaper is an optional auxiliary traffic shaper that can be | ||
| // used to manage the bandwidth of the link. | ||
|
|
@@ -3163,11 +3163,11 @@ func (l *channelLink) processRemoteAdds(fwdPkg *channeldb.FwdPkg) { | |
| continue | ||
| } | ||
|
|
||
| accountableValue := l.experimentalAccountability( | ||
| record.CustomSet(add.CustomRecords), | ||
| ) | ||
| accountableType := uint64( | ||
| lnwire.ExperimentalAccountableType, | ||
| ) | ||
|
|
||
| switch fwdPkg.State { | ||
|
|
@@ -3191,9 +3191,9 @@ func (l *channelLink) processRemoteAdds(fwdPkg *channeldb.FwdPkg) { | |
| BlindingPoint: fwdInfo.NextBlinding, | ||
| } | ||
|
|
||
| accountableValue.WhenSome(func(e byte) { | ||
| custRecords := map[uint64][]byte{ | ||
| accountableType: {e}, | ||
| } | ||
|
|
||
| outgoingAdd.CustomRecords = custRecords | ||
|
|
@@ -3249,9 +3249,9 @@ func (l *channelLink) processRemoteAdds(fwdPkg *channeldb.FwdPkg) { | |
| BlindingPoint: fwdInfo.NextBlinding, | ||
| } | ||
|
|
||
| accountableValue.WhenSome(func(e byte) { | ||
| addMsg.CustomRecords = map[uint64][]byte{ | ||
| accountableType: {e}, | ||
| } | ||
| }) | ||
|
|
||
|
|
@@ -3340,44 +3340,42 @@ func (l *channelLink) processRemoteAdds(fwdPkg *channeldb.FwdPkg) { | |
| l.forwardBatch(reforward, switchPackets...) | ||
| } | ||
|
|
||
| // experimentalAccountability returns the value to set for our outgoing | ||
| // experimental accountable field. It only considers the accountability bit, | ||
| // other custom records present are not considered for forwarding. | ||
| func (l *channelLink) experimentalAccountability( | ||
|
Collaborator
There was a problem hiding this comment. are we ok that basically this check resets the whole CustomRecord froward TLV in case no Accuntability bit is set? Probably not a problem now since it is not used for anything but I think this should be more fine granular in the future ? This seems to remove the old customRecord data and replaces it with the single lnwire.ExperimentalUnaccountable value can you verifiy @gemini-code-assist There was a problem hiding this comment. You've accurately identified a potential issue, @ziggie1984. The current implementation of This means any other custom records present in the incoming HTLC would be lost when forwarding. To prevent this, the You can use the For lines 3194-3199: accountableValue.WhenSome(func(e byte) {
// Create a mutable copy of the incoming custom records.
custRecords := add.CustomRecords.Copy()
custRecords[accountableType] = []byte{e}
outgoingAdd.CustomRecords = custRecords
})And similarly for lines 3252-3255: accountableValue.WhenSome(func(e byte) {
// Create a mutable copy of the incoming custom records.
custRecords := add.CustomRecords.Copy()
custRecords[accountableType] = []byte{e}
addMsg.CustomRecords = custRecords
})This ensures that only the
Contributor
Author
There was a problem hiding this comment. could you clarify? I'm not seeing the custom records passed to
Collaborator
There was a problem hiding this comment. so currently I think we do not forward any customRecords so it is fine but look at the code, as soon as this option has a value: Line 3166 in 801de79 we would replace the complete custome records and in the above function we do not merge any custom records we just set the Endorse or Unendorse Bit: Line 3199 in 801de79
Collaborator
There was a problem hiding this comment. yeah I was referring to when it is set, we would overwrite all values which were there in the first place and set only the endorseBit/unendorse
Contributor
Author
There was a problem hiding this comment. ah, ok. As you said, seems fine given that at that point the Line 3187 in 801de79 CustomRecords not set. But lmk if you'd want me to change it
Collaborator
There was a problem hiding this comment. can you maybe add in the Can you also rebase! |
||
| customUpdateAdd record.CustomSet) fn.Option[byte] { | ||
|
|
||
| if !l.cfg.ShouldFwdExpAccountability() { | ||
| return fn.None[byte]() | ||
| } | ||
|
|
||
| // If we don't have any custom records or the experimental field is | ||
| // not set, just forward a zero value. | ||
| if len(customUpdateAdd) == 0 { | ||
| return fn.Some[byte](lnwire.ExperimentalUnaccountable) | ||
| } | ||
|
|
||
| t := uint64(lnwire.ExperimentalAccountableType) | ||
| value, set := customUpdateAdd[t] | ||
| if !set { | ||
| return fn.Some[byte](lnwire.ExperimentalUnaccountable) | ||
| } | ||
|
|
||
| // We expect at least one byte for this field, consider it invalid if | ||
| // it has no data and just forward a zero value. | ||
| if len(value) == 0 { | ||
| return fn.Some[byte](lnwire.ExperimentalUnaccountable) | ||
| } | ||
|
|
||
| // Only forward accountable if the incoming link is accountable. | ||
| if value[0] == lnwire.ExperimentalAccountable { | ||
| return fn.Some[byte](lnwire.ExperimentalAccountable) | ||
| } | ||
|
|
||
| // Forward as unaccountable otherwise, including cases where we've | ||
| // received an invalid value that uses more than 3 bits of information. | ||
| return fn.Some[byte](lnwire.ExperimentalUnaccountable) | ||
| } | ||
|
|
||
| // processExitHop handles an htlc for which this link is the exit hop. It | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: one mention of endorsement left in comment.