Skip to content

Conversation

@evansmj
Copy link
Contributor

@evansmj evansmj commented May 1, 2024

This commit adds an invoice rune to the ConnectWallet component. Most values in that screen come from the backend method /shared/connectwallet and the type is found at app-config.type.ts/WalletConnect.

An invoice rune is added to WalletConnect because that is what is shown on the ConnectWallet.tsx component. ApplicationActions.SET_WALLET_CONNECT appends an invoice rune if one is found. If one is not found, ConnectWallet.tsx has a useEffect that checks for a missing invoice rune, and makes a createrune rpc call and then refreshes the component.

This commit adds an invoice rune to the ConnectWallet component.

On application start, we try to read an INVOICE_RUNE from .commando-env. One will be written to .commando-env when the user clicks to create one, if none exist already. If one does exist, it gets passed through and shown on the screen.

ShahanaFarooqui and others added 30 commits October 26, 2023 14:42
zsh has an expansion with '='.
if you run `source ./env.sh` in zsh, it will error `zsh: = not found`.
Using [[ ]] in the if statement is safer and results in`=` evaluating
properly with [[ ]] conditionals.
Fix README.md typos
This commit adds react-router so that we can route to other pages, such as the upcoming Bookkeeper Dashboard page.
It is set up so that the Header is shared among both the initial aka /home screen and the /bookkeeper screen. A redirect for '/' to '/home/' was added to support this shared header. Tests were written in App.test.tsx for this.

There is no discernible difference to a user with this change, other than if they manually navigate to /bookkeeper they will see a blank page. There is no button navigating to /bookkeeper and will not be until it is ready for production. This is so that regular pull request reviews can be done vs having to review a giant pull request with the new bookkeeper dashboard feature.

Test dependencies were updated and ts-jest + axios-mock-adapter were added so that components that import axios could be rendered in tests.
- Broke CLNHome and Bookkeeper into their own components
- Keeping App smaller with Root component and root router only
- Undo typescript downgrade from v5 to v4.
- Adding env.sh to gitignore to avoid its future commits
The "no transactions helper text" displays two different strings depending on if the node has active channels or not.  This commit fixes a bug where these two are backwards and adds tests in CLNTransactionsList.test.tsx.
…tion-channels-text

Fix empty transactions channel helper text
Adds tests for many components.  npm run test should result in all tests passing.
- Fixed DateBox test with locale
- Fixed timer and canvas warnings
- Moved mock data into utilities/test-utilities
…n-fix

Adding local env.sh and indentation fixes
Renamed the type Channels to ChannelsDomain since it has extra fields added.  ChannelsDto is the response from the network.  This was done so it is clear what comes from the network and what is calculated, and to map fields from listpeerchannels into the previous Channel domain model.  Removed some fields that are not used and not available in listpeerchannels: direction, max_htlc_value_in_flight_msat, htlc_minimum_msat, last_tx_fee_msat.  Other fields were duplicates such as spendable_msatoshi and deleted since spendable_msat exists in listpeerchannels as well as in the original Channels object.

use-http sendRequestToStore is now able to handle multiple rpc methods and it bundles the responses together.  This allows for sending 2+ rpc calls to 1 state, such as for the new channels AppContextType.

Removed the controllers/lightning.ts behavior of checking if listpeers was called and adding node.alias to the response.  This was so that the domain layer would handle setting domain data.

channels was added as an AppContextType.  It combines listpeerchannels and listnodes so that the node aliases can be set to the channels.

Added comment to isCompatibleVersion to explain what it does.

Added Channels component tests.  Update mock store to use channels instead of listChannels.
The code is written as per newer version on CLN.

For older CLN nodes, we will collect channels arrays from listPeers.peers array and convert them into newer channels type by adding id as peer_id and connected and peer_connected. Remaining data stays the same.
For eg. changed `satoshi_to_us` to `to_us_sat`, `satoshi_to_them` to `to_them_sat` etc.
…listpeerchannels

Update to listpeerchannels
…data

Updated the mock data and checks with more realistic dataset
…emoval

Remove msatoshi_received, msatoshi, msatoshi_sent from Payments and I…
…-compatble

RC version compatibility check
@evansmj
Copy link
Contributor Author

evansmj commented May 1, 2024

I refactored sendRequestToSetStore to take Request as a parameter, so that I could bundle api calls that use different urls or methods, and send them to the same callback, such as for:

sendRequestToSetStore(
      appCtx.setWalletConnect, 
      { method: 'get', url: '/shared/connectwallet' },
      { method: 'post', url: '/cln/call', body: { method: 'showrunes' } });

@evansmj evansmj force-pushed the evansmj/invoice-rune-component branch from 1c48653 to 309a599 Compare May 7, 2024 23:50
@ShahanaFarooqui
Copy link
Collaborator

Hi @evansmj, Thank you for the PR.

Here are my inputs after reviewing it:

  • Lint errors: There are two lint errors (use-http.ts Line 114 & ConnectWallet.tsx Line 51). The first one can be removed by adding missing getConnectWallet dependency in the array. For connectwallet missing dependency error, adding // eslint-disable-next-line react-hooks/exhaustive-deps will disable it for the linter.

  • New Invoice rune on every refresh: Currently createInvoiceRune is not dependent upon showrunes response; resulting in a new invoice rune creation on every refresh/startup. I ended up with 120 new invoice rune while testing this PR :).

  • Generating a rune without user action: As runes are directly related to node security, it would be better to generate it with user's active involvement. For that, we can leave the box empty if the rune doesn't exist already with Add button. And generate the rune only if user clicks on the button (image below).

  • Showrunes: Calling showrunes in the frontend will expose all runes from the node to the browser. I would avoid calling it in the backend also as we are already using another file (env COMMANDO_CONFIG) to get the master rune from. We can save INVOICE_RUNE along with LIGHTNING_PUBKEY and LIGHTNING_RUNE in the same file.

  • Invoice rune persistance: Currently, newly generate invoice rune only persists in the frontend. It would be better if we can keep the state persistent in backend & frontend both.

In summary, invoice rune user story should look like:

1: On application start, read the INVOICE_RUNE value from COMMANDO_CONFIG.
2: Set the value in backend's CONNECT_WALLET_SETTINGS either by step 1 or set to empty string if missing.
3: Show invoice rune value on the connect wallet screen. Show Add button with tooltip if it is empty otherwise simple copy button.
4: On Add button click, call create rune from the frontend.
5: Once the rune value in received in the backend, save that in the COMMANDO_CONFIG.
6: Also set it in the backend (APP_CONSTANTS?, CONNECT_WALLET_SETTINGS?, lightning.service.ts's constructor(), etc.).
7: Send the whole connectWallet response back to the UI and update the UI with Invoice rune value.

Screenshot from 2024-05-16 16-17-21

@evansmj
Copy link
Contributor Author

evansmj commented May 28, 2024

Hey @ShahanaFarooqui im looking through figma and the existing app but i can't find a nice + icon svg. Is there one you have in mind? otherwise i have this one:

Screenshot 2024-05-27 at 9 36 37 PM
Screenshot 2024-05-27 at 9 36 47 PM

@ShahanaFarooqui
Copy link
Collaborator

It looks fine but I would recommend to tag Basak on figma asking for plus icon svg. It will save us some time and reworking effort.

@evansmj
Copy link
Contributor Author

evansmj commented May 28, 2024

It looks fine but I would recommend to tag Basak on figma asking for plus icon svg. It will save us some time and reworking effort.

Sure i've added the request here
https://www.figma.com/design/09CUihPE1P6CDf7KlqTExu?node-id=203-2197#818167883

This commit adds an invoice rune to the ConnectWallet component.

On application start, we try to read an INVOICE_RUNE from .commando-env.  One will be written to .commando-env when the user clicks to create one, if none exist already.  If one does exist, it gets passed through and shown on the screen.
@evansmj evansmj force-pushed the evansmj/invoice-rune-component branch from 3d78312 to 5b881c8 Compare May 31, 2024 02:30
@evansmj
Copy link
Contributor Author

evansmj commented May 31, 2024

hey @ShahanaFarooqui ready for review. I also added a loading state and disable the invoice rune buttons while loading. And I show toasts for successfully creating an invoice rune and if an error occurs.

- Added tooltip for `create new` button
- Added placeholder if the invoice rune in empty
- Changed spinner type to application's UI standard
- Added error detail in toast if creation fails
- Changed to "a bit" slicker svg for Add
@ShahanaFarooqui ShahanaFarooqui changed the base branch from Release-0.0.5 to Release-0.0.6 June 6, 2024 04:24
@ShahanaFarooqui ShahanaFarooqui force-pushed the evansmj/invoice-rune-component branch from af4d1c5 to 82e3a9c Compare June 6, 2024 05:43
@ShahanaFarooqui ShahanaFarooqui merged commit 4272cdc into ElementsProject:Release-0.0.6 Jun 6, 2024
@ShahanaFarooqui ShahanaFarooqui linked an issue Jun 6, 2024 that may be closed by this pull request
@john-zaprite
Copy link

Thanks everyone. Looking forward to integrating CLN soon!

ShahanaFarooqui added a commit that referenced this pull request Sep 7, 2024
* Updating version number

* Display Invoice Rune (#56)

* Display Invoice Rune

* Cosmetic updates for Invoice Rune

* Update plus/add icon svg (#70)

* Resolve commando-rune deprecation and use createrune instead (#72)

* Change invoice and offer to lowercase (#68)

* Add tests for address casing fix (#69)

* Update jsx camelCase property name

* Adding mock use-http hook

* Add tests for invoice and offer capital casing

---------

Co-authored-by: Michael Evans <evansmj@users.noreply.github.com>
Co-authored-by: afxsoftware <33781025+afxsoftware@users.noreply.github.com>
evansmj added a commit to evansmj/cln-application that referenced this pull request Dec 26, 2024
* Display Invoice Rune

This commit adds an invoice rune to the ConnectWallet component.

On application start, we try to read an INVOICE_RUNE from .commando-env.  One will be written to .commando-env when the user clicks to create one, if none exist already.  If one does exist, it gets passed through and shown on the screen.

* Cosmetic updates for Invoice Rune

- Added tooltip for `create new` button
- Added placeholder if the invoice rune in empty
- Changed spinner type to application's UI standard
- Added error detail in toast if creation fails
- Changed to "a bit" slicker svg for Add

---------

Co-authored-by: ShahanaFarooqui <shahana.farooqui@gmail.com>
evansmj added a commit to evansmj/cln-application that referenced this pull request Dec 26, 2024
* Display Invoice Rune

This commit adds an invoice rune to the ConnectWallet component.

On application start, we try to read an INVOICE_RUNE from .commando-env.  One will be written to .commando-env when the user clicks to create one, if none exist already.  If one does exist, it gets passed through and shown on the screen.

* Cosmetic updates for Invoice Rune

- Added tooltip for `create new` button
- Added placeholder if the invoice rune in empty
- Changed spinner type to application's UI standard
- Added error detail in toast if creation fails
- Changed to "a bit" slicker svg for Add

---------

Co-authored-by: ShahanaFarooqui <shahana.farooqui@gmail.com>
ShahanaFarooqui added a commit to evansmj/cln-application that referenced this pull request Feb 25, 2025
* Display Invoice Rune

This commit adds an invoice rune to the ConnectWallet component.

On application start, we try to read an INVOICE_RUNE from .commando-env.  One will be written to .commando-env when the user clicks to create one, if none exist already.  If one does exist, it gets passed through and shown on the screen.

* Cosmetic updates for Invoice Rune

- Added tooltip for `create new` button
- Added placeholder if the invoice rune in empty
- Changed spinner type to application's UI standard
- Added error detail in toast if creation fails
- Changed to "a bit" slicker svg for Add

---------

Co-authored-by: ShahanaFarooqui <shahana.farooqui@gmail.com>
ShahanaFarooqui added a commit that referenced this pull request Mar 13, 2025
* Display Invoice Rune

This commit adds an invoice rune to the ConnectWallet component.

On application start, we try to read an INVOICE_RUNE from .commando-env.  One will be written to .commando-env when the user clicks to create one, if none exist already.  If one does exist, it gets passed through and shown on the screen.

* Cosmetic updates for Invoice Rune

- Added tooltip for `create new` button
- Added placeholder if the invoice rune in empty
- Changed spinner type to application's UI standard
- Added error detail in toast if creation fails
- Changed to "a bit" slicker svg for Add

---------

Co-authored-by: ShahanaFarooqui <shahana.farooqui@gmail.com>
ShahanaFarooqui added a commit to evansmj/cln-application that referenced this pull request May 7, 2025
* Display Invoice Rune

This commit adds an invoice rune to the ConnectWallet component.

On application start, we try to read an INVOICE_RUNE from .commando-env.  One will be written to .commando-env when the user clicks to create one, if none exist already.  If one does exist, it gets passed through and shown on the screen.

* Cosmetic updates for Invoice Rune

- Added tooltip for `create new` button
- Added placeholder if the invoice rune in empty
- Changed spinner type to application's UI standard
- Added error detail in toast if creation fails
- Changed to "a bit" slicker svg for Add

---------

Co-authored-by: ShahanaFarooqui <shahana.farooqui@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Invoice Macaroon to Settings

3 participants