-
Notifications
You must be signed in to change notification settings - Fork 40
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add the option to mint public notes with the faucet #339
Conversation
Which type of note (public or private) do you think should be selected by default? |
bin/faucet/src/static/index.js
Outdated
downloadBlob(blob, 'note.mno'); | ||
} | ||
|
||
const noteId = response.headers.get('Content-Disposition').split('filename=')[1].replace(/"/g, ''); |
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.
This looks a bit complicated but also it might be fine for now. I guess an alternative would be for the name could be sent directly through a custom header to make it a bit simpler.
Alternatively the server could return a JSON response that contains the type of response, and (depending on the user selection) the encoded note details.
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.
Done! Didn't know about custom headers.
One suggestions: I would make the amount selectable (and maybe give a few options there), but would make public note and private notes into separate button. So, basically:
|
If this is done, we should validate these amounts on the server side so no one consumes an arbitrary amount of tokens, shouldn't we? |
I would just hard-code a few amounts and not allow free entry (mostly for simplicity). But I guess we will need to have a check on the server side if we send amounts with the request. |
Done! The new screenshots are pasted in the PR description.
I added the possible options to the |
Looks great! Thank you! Probably not for this PR: we should add some basic explanations about what public vs. private notes are (maybe on hover of the respective buttons, but maybe in some other way). |
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.
LGTM! Tested locally and had no issues. I left a small comment regarding the commands used to consume the note.
Also on a side note, now we'll need to keep in mind that changes to the client might impact here as well
Co-authored-by: Martin Fraga <[email protected]>
@tomyrd - oh, and let's update changelog. |
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.
Agreed, let's merge after the changelog is updated.
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.
agreed on the changelog
…n/miden-node into tomyrd-faucet-public-notes
@@ -2,6 +2,7 @@ | |||
|
|||
## 0.2.1 (2024-04-27) | |||
|
|||
* Added option to mint pulic notes in the faucet (#339). |
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.
This should go into v0.3.0
section (you may need to rebase from the latest next
).
The tests are failing because of the recent changes in |
Great job @tomyrd, love the new UI. One thing I believe that this PR has removed the loading spinner when the user clicks on one of the buttons. This creates a UX where the user does not know if the app is loading waiting for a response from the rollup or if the website is broken. I believe that we should add it back in another PR. |
Done. The PR is #362 . |
closes #329
The previous faucet only minted private notes which had to be downloaded and imported into the client. Now with the implementation of public notes the flow can be much simpler. Just mint the note, sync and consume. This PR adds the option to mint public notes with the faucet and adds some useful information about how to consume it.
The initial screen looks like this:
After minting a private note this information box is showed:
If the note is public the commands are slightly different: