Update API responses to specify BtcAmount unit

t-bast commented 3 years ago

When we return a bitcoin amount, our JSON responses currently specify the unit (btc, sat, msat) in the parameter name, but it's a bit ugly and is missing in some places.

Maybe it would be better to include the unit explicitly in the response. I see two different options:

Instead of returning a numeric value, return a string including the unit (e.g. "2500 msat", "0.45 btc", "1.5 mbtc"). The drawback is that the string must be parsed to extract the amount (but it's trivial to do).
Instead of returning a numeric value, return an object with a unit field (e.g. {"amount": 2500, "unit": "msat"}, {"amount": 0.45, "unit": "btc"}, {"amount": 1.5, "unit": "mbtct"})

This would also be desirable for inputs, we could use strings with a unit suffix (e.g. `fundingFeerate="2500 sat/byte").

Harshil-Gupta commented 1 year ago

Hi, Is this issue still open? I am setting up the local environment and after that, I think I will be able to solve this issue.

t-bast commented 1 year ago

Sure, this issue is still open. Feel free to take a stab at it!

SiddheshKanawade commented 1 year ago

Hello @t-bast,

I am Siddhesh Kanawade, a third year undergraduate student from IIT Gandhinagar, India. I want to contribute to Eclair as a part of Summer of Bitcoin this year.

I participated in Google Summer of Code 2022[GSoC ‘22] under Casbin organization to contribute to casbin-rs, which is an implementation of Core Casbin in Rust. I integrated Casbin-rs with Axum middleware and wrote real-life examples and unit tests on it. Apart from this, I maintained their stale repositories and updated them to Rust 2021. You can find my detailed report about my contributions here. I also worked as a backend developer at Granular AI where I contributed in Python to build multiple microservices for Granular’s Geoengine Platform.

I want to contribute by integrating lnprototests testing framework for Eclair Lightning network. I have been following lnprototest's repository for a while and trying to understand its integration with core-lightning. I have been setting up Eclair in my local environment, currently I have some issues but I am pretty sure that I will be able to resolve them. I want to start contributing to Eclair by resolving this issue, could you please assign me this issue? Also, do we have any community channel for Eclair apart from Discord, it seems mentors are not that active over there.

Thank you.

SiddheshKanawade commented 1 year ago

@t-bast can you please assign me this issue.

Also, I have been trying to set the local environment for Eclair-core. It seems I am unable to establish the connection to some remote server. There might be good possibility that I may not be using correct config, but I am not sure how to resolve it. Following is the Stack trace:

[java.net.SocketException: Connection reset, java.net.SocketException: Connection reset, java.net.SocketException: Connection reset, java.net.ConnectException: Failed to connect to api.blockcypher.com/2606:4700:10:0:0:0:ac43:258:443, java.net.ConnectException: Failed to connect to api.blockcypher.com/2606:4700:10:0:0:0:6814:14fb:443]
- fetch latest block headers *** FAILED ***
  java.lang.AssertionError: Timeout (15 seconds) during expectMessageClass waiting for class fr.acinq.eclair.blockchain.watchdogs.BlockchainWatchdog$LatestHeaders
  at akka.actor.testkit.typed.internal.TestProbeImpl.assertFail(TestProbeImpl.scala:399)
  at akka.actor.testkit.typed.internal.TestProbeImpl.expectMessageClass_internal(TestProbeImpl.scala:239)
  at akka.actor.testkit.typed.internal.TestProbeImpl.expectMessageType(TestProbeImpl.scala:218)
  at fr.acinq.eclair.blockchain.watchdogs.ExplorerApiSpec.$anonfun$new$2(ExplorerApiSpec.scala:38)
  at scala.collection.immutable.List.foreach(List.scala:333)
  at fr.acinq.eclair.blockchain.watchdogs.ExplorerApiSpec.$anonfun$new$1(ExplorerApiSpec.scala:34)
  at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.scala:18)
  at org.scalatest.OutcomeOf.outcomeOf(OutcomeOf.scala:85)
  at org.scalatest.OutcomeOf.outcomeOf$(OutcomeOf.scala:83)
  at org.scalatest.OutcomeOf$.outcomeOf(OutcomeOf.scala:104)

System Information: OS = Ubuntu 22.04 Ram = 8 GB java --version = openjdk 11.0.18 2023-01-17 javac = javac 11.0.18 scala --version = 3.2.2

Stack Overflow link for similar trace Any Idea on how can I resolve the error?

t-bast commented 1 year ago

Hi @SiddheshKanawade, thanks for your interest in eclair! We do not assign issues to external contributors unless they have opened a pull request that fixes the issue. Don't hesitate to start working on this, and open a pull request once you have a first working prototype.

Also, do we have any community channel for Eclair apart from Discord

Please read our README and contribution guidelines, this is explained there.

Also, I have been trying to set the local environment for Eclair-core. It seems I am unable to establish the connection to some remote server. There might be good possibility that I may not be using correct config, but I am not sure how to resolve it. Following is the Stack trace:

This doesn't prevent you from working with eclair, it's only one of the tests that isn't passing. This test is explicitly marked with TestTags.ExternalApi because it calls an external server, and may thus fail if you have connectivity issues with that server. You can either resolve the connectivity issue (that's related to your setup, we can't help you much here) or simply ignore that test, it has no impact on the work you want to do.

SiddheshKanawade commented 1 year ago

@t-bast Thank you for the clarity. As of now I will bypass the test and try to resolve the this issue.

Also, as part of setting local environment for development, what are minimum required things to work? How can I ensure that my local environment is set correctly to meet the minimum requirement for development. This will enable me to view the changes that I make locally before committing to GitHub

PrathmeshRanjan commented 1 year ago

Hello @t-bast , I'm Prathmesh, one of the SOB'23 applicants. I would love to contribute to eclair in the projects "Lightning Accounting Tool for Eclair" and "Improve Lightning Network gossip". I have a basic workflow ready for the implementation of the projects. I would love to get it reviewed by you and receive your feedback. Is there any way through which we can communicate? Thank you and have a nice day!

claddyy commented 1 year ago

Hi @t-bast, I think we can solve this issue by defining a case class for the amount with an amount field and a unit field.

t-bast commented 1 year ago

I think we can solve this issue by defining a case class for the amount with an amount field and a unit field.

This is already the case, we use a case class for any type of amount (look at the MilliSatoshi and Satoshi classes in the codebase). It's just a matter of changing how those are encoded in the API responses, and how to handle backwards-compatibility.

ACINQ / eclair

Update API responses to specify BtcAmount unit #1858