Skip to content

Commit

Permalink
make short_channel_id optional
Browse files Browse the repository at this point in the history
short_channel_id is only set when the channel is confirmed onchain. This commit
makes the short_channel_id optional, to match reality.
  • Loading branch information
JssDWt committed Jan 22, 2024
1 parent 662558f commit 55e08fa
Show file tree
Hide file tree
Showing 10 changed files with 105 additions and 33 deletions.
2 changes: 1 addition & 1 deletion libs/sdk-bindings/src/breez_sdk.udl
Original file line number Diff line number Diff line change
Expand Up @@ -293,9 +293,9 @@ dictionary LnPaymentDetails {
};

dictionary ClosedChannelPaymentDetails {
string short_channel_id;
ChannelState state;
string funding_txid;
string? short_channel_id;
string? closing_txid;
};

Expand Down
15 changes: 11 additions & 4 deletions libs/sdk-core/src/breez_services.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2042,12 +2042,19 @@ impl Receiver for PaymentReceiver {
.find(|&c| c.state == ChannelState::Opened)
.ok_or_else(|| anyhow!("No open channel found"))?;
let hint = active_channel
.clone()
.alias_remote
.unwrap_or(active_channel.clone().short_channel_id);
.clone()
.or(active_channel.short_channel_id.clone());

short_channel_id = match hint {
None => None,
Some(hint) => {
let scid = parse_short_channel_id(&hint)?;
info!("Found channel ID: {scid:?} {active_channel:?}");
Some(scid)
}
};

short_channel_id = Some(parse_short_channel_id(&hint)?);
info!("Found channel ID: {short_channel_id:?} {active_channel:?}");
break;
}
}
Expand Down
2 changes: 1 addition & 1 deletion libs/sdk-core/src/bridge_generated.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1044,9 +1044,9 @@ impl rust2dart::IntoIntoDart<CheckMessageResponse> for CheckMessageResponse {
impl support::IntoDart for ClosedChannelPaymentDetails {
fn into_dart(self) -> support::DartAbi {
vec![
self.short_channel_id.into_into_dart().into_dart(),
self.state.into_into_dart().into_dart(),
self.funding_txid.into_into_dart().into_dart(),
self.short_channel_id.into_dart(),
self.closing_txid.into_dart(),
]
.into_dart()
Expand Down
6 changes: 2 additions & 4 deletions libs/sdk-core/src/greenlight/node_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1810,7 +1810,7 @@ impl From<cln::ListpeersPeersChannels> for Channel {
};

Channel {
short_channel_id: c.short_channel_id.unwrap_or_default(),
short_channel_id: c.short_channel_id,
state,
funding_txid: c.funding_txid.map(hex::encode).unwrap_or_default(),
spendable_msat: c.spendable_msat.unwrap_or_default().msat,
Expand Down Expand Up @@ -1884,9 +1884,7 @@ impl TryFrom<ListclosedchannelsClosedchannels> for Channel {
// To keep the conversion simple and fast, some closing-related fields (closed_at, closing_txid)
// are left empty here in the conversion, but populated later (via chain service lookup, or DB lookup)
Ok(Channel {
short_channel_id: c
.short_channel_id
.ok_or(anyhow!("short_channel_id is missing"))?,
short_channel_id: c.short_channel_id,
state: ChannelState::Closed,
funding_txid: hex::encode(c.funding_txid),
spendable_msat: c
Expand Down
4 changes: 2 additions & 2 deletions libs/sdk-core/src/models.rs
Original file line number Diff line number Diff line change
Expand Up @@ -727,9 +727,9 @@ pub struct LnPaymentDetails {
/// Represents the funds that were on the user side of the channel at the time it was closed.
#[derive(PartialEq, Eq, Debug, Clone, Deserialize, Serialize)]
pub struct ClosedChannelPaymentDetails {
pub short_channel_id: String,
pub state: ChannelState,
pub funding_txid: String,
pub short_channel_id: Option<String>,
/// Can be empty for older closed channels.
pub closing_txid: Option<String>,
}
Expand Down Expand Up @@ -1114,7 +1114,7 @@ impl OpeningFeeParamsMenu {
#[derive(Clone, PartialEq, Eq, Debug)]
pub struct Channel {
pub funding_txid: String,
pub short_channel_id: String,
pub short_channel_id: Option<String>,
pub state: ChannelState,
pub spendable_msat: u64,
pub receivable_msat: u64,
Expand Down
67 changes: 61 additions & 6 deletions libs/sdk-core/src/persist/channels.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ fn test_simple_sync_channels() {
let channels = vec![
Channel {
funding_txid: "123".to_string(),
short_channel_id: "10x11x12".to_string(),
short_channel_id: Some("10x11x12".to_string()),
state: ChannelState::Opened,
spendable_msat: 100,
receivable_msat: 1000,
Expand All @@ -163,7 +163,7 @@ fn test_simple_sync_channels() {
},
Channel {
funding_txid: "456".to_string(),
short_channel_id: "13x14x15".to_string(),
short_channel_id: Some("13x14x15".to_string()),
state: ChannelState::Opened,
spendable_msat: 200,
receivable_msat: 2000,
Expand All @@ -185,6 +185,61 @@ fn test_simple_sync_channels() {
assert_eq!(channels, queried_channels);
}

#[test]
fn test_sync_none_short_channel_id() {
use crate::persist::test_utils;

let storage = SqliteStorage::new(test_utils::create_test_sql_dir());

storage.init().unwrap();
let channels = vec![
Channel {
funding_txid: "123".to_string(),
short_channel_id: None,
state: ChannelState::Opened,
spendable_msat: 100,
receivable_msat: 1000,
closed_at: None,
funding_outnum: None,
alias_local: None,
alias_remote: None,
closing_txid: None,
htlcs: Vec::new(),
},
Channel {
funding_txid: "456".to_string(),
short_channel_id: None,
state: ChannelState::Closed,
spendable_msat: 200,
receivable_msat: 2000,
closed_at: None,
funding_outnum: None,
alias_local: None,
alias_remote: None,
closing_txid: None,
htlcs: Vec::new(),
},
];

storage.update_channels(&channels).unwrap();
let queried_channels = storage.list_channels().unwrap();
assert_eq!(channels, queried_channels);
assert!(queried_channels[0].short_channel_id.is_none());
assert!(queried_channels[1].short_channel_id.is_none());

// The short_channel_id is set once the funding tx has been confirmed.
// Make sure the short_channel_id is set after this update.
let mut channels = channels.clone();
channels[0].short_channel_id = Some("10x11x12".to_string());
channels[1].short_channel_id = Some("13x14x15".to_string());

storage.update_channels(&channels).unwrap();
let queried_channels = storage.list_channels().unwrap();
assert_eq!(channels, queried_channels);
assert!(queried_channels[0].short_channel_id.is_some());
assert!(queried_channels[1].short_channel_id.is_some());
}

#[test]
fn test_sync_closed_channels() {
use crate::persist::test_utils;
Expand All @@ -195,7 +250,7 @@ fn test_sync_closed_channels() {
let channels = vec![
Channel {
funding_txid: "123".to_string(),
short_channel_id: "10x11x12".to_string(),
short_channel_id: Some("10x11x12".to_string()),
state: ChannelState::Opened,
spendable_msat: 100,
receivable_msat: 1000,
Expand All @@ -209,7 +264,7 @@ fn test_sync_closed_channels() {
// Simulate closed channel that was persisted with closed_at and closing_txid
Channel {
funding_txid: "456".to_string(),
short_channel_id: "13x14x15".to_string(),
short_channel_id: Some("13x14x15".to_string()),
state: ChannelState::Closed,
spendable_msat: 200,
receivable_msat: 2000,
Expand Down Expand Up @@ -239,7 +294,7 @@ fn test_sync_closed_channels() {
let expected = vec![
Channel {
funding_txid: "123".to_string(),
short_channel_id: "10x11x12".to_string(),
short_channel_id: Some("10x11x12".to_string()),
state: ChannelState::Closed,
spendable_msat: 100,
receivable_msat: 1000,
Expand All @@ -252,7 +307,7 @@ fn test_sync_closed_channels() {
},
Channel {
funding_txid: "456".to_string(),
short_channel_id: "13x14x15".to_string(),
short_channel_id: Some("13x14x15".to_string()),
state: ChannelState::Closed,
spendable_msat: 200,
receivable_msat: 2000,
Expand Down
10 changes: 5 additions & 5 deletions libs/sdk-flutter/lib/bridge_generated.dart
Original file line number Diff line number Diff line change
Expand Up @@ -436,17 +436,17 @@ class CheckMessageResponse {

/// Represents the funds that were on the user side of the channel at the time it was closed.
class ClosedChannelPaymentDetails {
final String shortChannelId;
final ChannelState state;
final String fundingTxid;
final String? shortChannelId;

/// Can be empty for older closed channels.
final String? closingTxid;

const ClosedChannelPaymentDetails({
required this.shortChannelId,
required this.state,
required this.fundingTxid,
this.shortChannelId,
this.closingTxid,
});
}
Expand Down Expand Up @@ -2975,9 +2975,9 @@ class BreezSdkCoreImpl implements BreezSdkCore {
final arr = raw as List<dynamic>;
if (arr.length != 4) throw Exception('unexpected arr length: expect 4 but see ${arr.length}');
return ClosedChannelPaymentDetails(
shortChannelId: _wire2api_String(arr[0]),
state: _wire2api_channel_state(arr[1]),
fundingTxid: _wire2api_String(arr[2]),
state: _wire2api_channel_state(arr[0]),
fundingTxid: _wire2api_String(arr[1]),
shortChannelId: _wire2api_opt_String(arr[2]),
closingTxid: _wire2api_opt_String(arr[3]),
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -319,17 +319,25 @@ fun asClosedChannelPaymentDetails(closedChannelPaymentDetails: ReadableMap): Clo
if (!validateMandatoryFields(
closedChannelPaymentDetails,
arrayOf(
"shortChannelId",
"state",
"fundingTxid",
),
)
) {
return null
}
val shortChannelId = closedChannelPaymentDetails.getString("shortChannelId")!!
val state = closedChannelPaymentDetails.getString("state")?.let { asChannelState(it) }!!
val fundingTxid = closedChannelPaymentDetails.getString("fundingTxid")!!
val shortChannelId =
if (hasNonNullKey(
closedChannelPaymentDetails,
"shortChannelId",
)
) {
closedChannelPaymentDetails.getString("shortChannelId")
} else {
null
}
val closingTxid =
if (hasNonNullKey(
closedChannelPaymentDetails,
Expand All @@ -341,18 +349,18 @@ fun asClosedChannelPaymentDetails(closedChannelPaymentDetails: ReadableMap): Clo
null
}
return ClosedChannelPaymentDetails(
shortChannelId,
state,
fundingTxid,
shortChannelId,
closingTxid,
)
}

fun readableMapOf(closedChannelPaymentDetails: ClosedChannelPaymentDetails): ReadableMap {
return readableMapOf(
"shortChannelId" to closedChannelPaymentDetails.shortChannelId,
"state" to closedChannelPaymentDetails.state.name.lowercase(),
"fundingTxid" to closedChannelPaymentDetails.fundingTxid,
"shortChannelId" to closedChannelPaymentDetails.shortChannelId,
"closingTxid" to closedChannelPaymentDetails.closingTxid,
)
}
Expand Down
14 changes: 9 additions & 5 deletions libs/sdk-react-native/ios/BreezSDKMapper.swift
Original file line number Diff line number Diff line change
Expand Up @@ -337,9 +337,6 @@ enum BreezSDKMapper {
}

static func asClosedChannelPaymentDetails(closedChannelPaymentDetails: [String: Any?]) throws -> ClosedChannelPaymentDetails {
guard let shortChannelId = closedChannelPaymentDetails["shortChannelId"] as? String else {
throw SdkError.Generic(message: errMissingMandatoryField(fieldName: "shortChannelId", typeName: "ClosedChannelPaymentDetails"))
}
guard let stateTmp = closedChannelPaymentDetails["state"] as? String else {
throw SdkError.Generic(message: errMissingMandatoryField(fieldName: "state", typeName: "ClosedChannelPaymentDetails"))
}
Expand All @@ -348,6 +345,13 @@ enum BreezSDKMapper {
guard let fundingTxid = closedChannelPaymentDetails["fundingTxid"] as? String else {
throw SdkError.Generic(message: errMissingMandatoryField(fieldName: "fundingTxid", typeName: "ClosedChannelPaymentDetails"))
}
var shortChannelId: String?
if hasNonNilKey(data: closedChannelPaymentDetails, key: "shortChannelId") {
guard let shortChannelIdTmp = closedChannelPaymentDetails["shortChannelId"] as? String else {
throw SdkError.Generic(message: errUnexpectedValue(fieldName: "shortChannelId"))
}
shortChannelId = shortChannelIdTmp
}
var closingTxid: String?
if hasNonNilKey(data: closedChannelPaymentDetails, key: "closingTxid") {
guard let closingTxidTmp = closedChannelPaymentDetails["closingTxid"] as? String else {
Expand All @@ -357,18 +361,18 @@ enum BreezSDKMapper {
}

return ClosedChannelPaymentDetails(
shortChannelId: shortChannelId,
state: state,
fundingTxid: fundingTxid,
shortChannelId: shortChannelId,
closingTxid: closingTxid
)
}

static func dictionaryOf(closedChannelPaymentDetails: ClosedChannelPaymentDetails) -> [String: Any?] {
return [
"shortChannelId": closedChannelPaymentDetails.shortChannelId,
"state": valueOf(channelState: closedChannelPaymentDetails.state),
"fundingTxid": closedChannelPaymentDetails.fundingTxid,
"shortChannelId": closedChannelPaymentDetails.shortChannelId == nil ? nil : closedChannelPaymentDetails.shortChannelId,
"closingTxid": closedChannelPaymentDetails.closingTxid == nil ? nil : closedChannelPaymentDetails.closingTxid,
]
}
Expand Down
2 changes: 1 addition & 1 deletion libs/sdk-react-native/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,9 @@ export type CheckMessageResponse = {
}

export type ClosedChannelPaymentDetails = {
shortChannelId: string
state: ChannelState
fundingTxid: string
shortChannelId?: string
closingTxid?: string
}

Expand Down

0 comments on commit 55e08fa

Please sign in to comment.