BYO network transport when minting tokens #605

vnprc · 2025-02-17T20:39:57Z

Description

Hashpool operates on a slightly different model from typical ecash mints. It generates ecash tokens atomically when validating mining pool shares. In order to accomplish atomicity we need to piggyback the ecash operations on existing mining protocol messages. This means the ecash mint and wallet will not be able to make synchronous HTTP calls during the process of minting new tokens, which is the default assumption in cdk.

Here's the new flow:

miner finds a valid share above the pool difficulty threshold
proxy/wallet generates a new premint secret and saves it to the wallet
proxy/wallet submits a mining share and premint secret to the pool/mint
pool validates the mining share
mint creates a new quote using the block header hash of the mining share as a quote ID (is this necessary or useful?)
mint signs the blinded secret
pool returns this signature in the 'mining share accepted' message payload
proxy receives the signature from the mint
proxy retrieves the blinded secret from the cashu wallet
wallet validates the signature and stores the proof as an ecash token

There are two main differences with this design:

the wallet must store keysets and blinded secrets locally
the wallet must rely on local information about the mint rather than querying the mint at will

Notes to the reviewers

I have exposed some helper functions to convert between Amount and AmountStr. This helps the wallet store blinded secrets using a BTreeMap. This is kind of a code smell (Why are we converting to a string for indexing purposes? Shouldn't the data structure handle this for us?) but it was the shortest path to victory during development. I'm not sure if this is necessary. Hoping to get some clarity in this PR.

I didn't discuss it in the minting flow above but mint keysets will be propagated by the pool in a mining message payload and stored locally in the wallet. This requires a new function to add mint keysets to the wallet without making any HTTP calls.

I have opened this PR in draft state with many TODO comments and macros. I will fix these issues in due time but I wanted to get feedback on my general approach.

Suggested CHANGELOG Updates

CHANGED

ADDED

REMOVED

FIXED

Checklist

I followed the code style guidelines
I ran just final-check before committing

- rename gen_ehash_premint_secrets to create_premint_secrets - rename gen_ehash_proofs to verify_and_store_proofs - make generate_premint_secrets private instead of public - add CurrencyUnit function param to create_premint_secrets - other code readability improvements

vnprc · 2025-02-17T20:46:30Z

crates/cdk/src/wallet/keysets.rs

+
+    /// Get active keyset for mint from local without querying the mint
+    #[instrument(skip(self))]
+    pub async fn get_active_mint_keyset_local(&self) -> Result<KeySetInfo, Error> {


Needs a better function name that indicates this function doesn't make any network calls.

vnprc · 2025-02-17T20:49:17Z

crates/cdk/src/wallet/mint.rs

I wrote these functions based on an old version of cdk and copied them over to the latest branch. Need to look back and make sure the code I copied hasn't changed in the mean time. Also, and related, I am uncomfortable with the amount of duplicated code in this module. Need to refactor the shared code between the existing mint function and these new functions.

average-gary · 2025-02-18T13:47:21Z

crates/cdk/src/cdk_database/wallet_memory.rs

@@ -28,6 +29,7 @@ pub struct WalletMemoryDatabase {
    proofs: Arc<RwLock<HashMap<PublicKey, ProofInfo>>>,
    keyset_counter: Arc<RwLock<HashMap<Id, u32>>>,
    nostr_last_checked: Arc<RwLock<HashMap<PublicKey, u32>>>,
+    premint_secrets: Arc<RwLock<HashMap<String, PreMintSecrets>>>,


Why String for the HashMap key here?

Honestly, I don't know if this is necessary. It would be great if I could get rid of the string entirely.

It works this way because I copied some code from somewhere else and it used a string as the index. This code comment seems to indicate using a string helps with sorting these items in a BTreeMap.

/// String wrapper for an [Amount]. /// /// It ser-/deserializes the inner [Amount] to a string, while at the same time using the [u64] /// value of the [Amount] for comparison and ordering. This helps automatically sort the keys of /// a [BTreeMap] when [AmountStr] is used as key. #[derive(Debug, Clone, PartialEq, Eq)] pub struct AmountStr(Amount);

We're planning to remove this memory db so i wouldn't worry about this.

#607

AmountStr is used because the amount in the keyset response is a string not an int. https://github.com/cashubtc/nuts/blob/main/02.md#requesting-public-keys-for-a-specific-keyset

I use AmountStr in hashpool in the same way. Is this the recommended way to put pubkeys in a BTreeMap?

https://github.com/vnprc/hashpool/blob/master/protocols/v2/subprotocols/mining/src/cashu.rs#L270

You can just use the Amount there no need to wrap it in the AmoutnStr. The reason we have to use AmountStr is for when the Keys response is json serialized it needs to be as a string and not an int, so that's really the only place we should use AmountStr everywhere else is Amount.

EDIT: I see you're creating a Keys which currently does expect the AmountStr

cdk/crates/cashu/src/nuts/nut01/mod.rs

Line 63 in e1458b0

pub fn new(keys: BTreeMap<AmountStr, PublicKey>) -> Self {

. But since whats needed is as i described above we should change this and that can be an amount and we can just create a custom serialize deserilization, that maybe a better way to handle this and then we don't need the AmountStr type?

cc @crodas

vnprc · 2025-02-19T15:18:57Z

crates/cdk/src/wallet/mint.rs

+        let count = self
+            .localstore
+            .get_keyset_counter(&active_keyset_id)
+            .await?
+            .unwrap_or(0)
+            + 1;


I think we have a race condition here where premint secrets could be generated twice here before incrementing the counter.

https://github.com/cashubtc/nuts/blob/main/13.md

@thesimplekid will find the exact PR and share here

We may want to change this to atomically increment the counter here instead of when constructing proofs.

unwrap_or(default) seems more rusty

increment counters when the secrets are generated (not when the proofs are constructed)

atomicity: race condition between read+write (get counter + bump counter), would require some locking mechanism

vnprc · 2025-02-19T15:23:34Z

crates/cdk/src/wallet/mint.rs

+        &self,
+        signatures: [Option<BlindSignature>; 64],
+        quote_id: &str,
+    ) -> Result<Amount, Error> {


Update this to return proofs instead of amount

vnprc · 2025-02-19T15:31:44Z

crates/cdk/src/wallet/mint.rs

+            mint_url: mint_url.parse()?,
+            amount: Amount::from(amount),
+            unit: currency_unit,
+            request: "todo".to_string(), // TODO: what does request do?


This doesn't really translate to our new flow because the "proof of payment" is generated before minting is even initiated. Use empty string for now.

We are pre-paying so we don't need a payment request.

vnprc · 2025-02-19T16:01:40Z

We already have types for bolt11 and bolt12 quotes. Create a new type for a mining share quote.

When minting eHash, submit the mining share and blinded secret. The mint will respond with a quote ID. Then any existing ecash wallet can mint ehash just by following the NUT04 protocol.

Optionally, also submit pubkey to lock the eHash token to.

vnprc · 2025-02-19T16:14:41Z

After team review we want to change the flow to work asynchronously within the NUT04 spec. It will work like this:

submit the mining share to the mint along with a pubkey (NUT20)
mint creates a quote and responds with the quote
mint updates the quote to paid when the mining share is validated
after this, any wallet with access to the corresponding privkey can redeem the quote for eHash as described in NUT04 & NUT20

This makes it easy to run a normal cashu mint and existing wallets can already support this. This decouples the mining protocol from the cashu protocol and lets us use sexy stuff like websockets without changing the mining protocol messages.

vnprc added 3 commits February 14, 2025 17:35

feat: BYO network transport

2023939

fix: retrieve currency unit from mint quote

b646927

vnprc commented Feb 17, 2025

View reviewed changes

vnprc mentioned this pull request Feb 18, 2025

cdk - BYO network transport PR vnprc/hashpool#15

Open

average-gary reviewed Feb 18, 2025

View reviewed changes

crodas mentioned this pull request Feb 19, 2025

Improve Amount and remove AmountStr. #609

Open

vnprc commented Feb 19, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BYO network transport when minting tokens #605

BYO network transport when minting tokens #605

vnprc commented Feb 17, 2025 •

edited

Loading

vnprc Feb 17, 2025

vnprc Feb 17, 2025

average-gary Feb 18, 2025

vnprc Feb 18, 2025 •

edited

Loading

thesimplekid Feb 18, 2025 •

edited

Loading

vnprc Feb 18, 2025

thesimplekid Feb 18, 2025 •

edited

Loading

vnprc Feb 19, 2025

vnprc Feb 19, 2025

vnprc Feb 19, 2025

callebtc Feb 19, 2025

vnprc Feb 19, 2025

vnprc Feb 19, 2025

vnprc Feb 19, 2025

vnprc commented Feb 19, 2025 •

edited

Loading

vnprc commented Feb 19, 2025 •

edited

Loading

BYO network transport when minting tokens #605

Are you sure you want to change the base?

BYO network transport when minting tokens #605

Conversation

vnprc commented Feb 17, 2025 • edited Loading

Description

Notes to the reviewers

Suggested CHANGELOG Updates

CHANGED

ADDED

REMOVED

FIXED

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vnprc Feb 18, 2025 • edited Loading

Choose a reason for hiding this comment

thesimplekid Feb 18, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thesimplekid Feb 18, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vnprc commented Feb 19, 2025 • edited Loading

vnprc commented Feb 19, 2025 • edited Loading

vnprc commented Feb 17, 2025 •

edited

Loading

vnprc Feb 18, 2025 •

edited

Loading

thesimplekid Feb 18, 2025 •

edited

Loading

thesimplekid Feb 18, 2025 •

edited

Loading

vnprc commented Feb 19, 2025 •

edited

Loading

vnprc commented Feb 19, 2025 •

edited

Loading