Skip to content
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

Add CashuWallet.checkProofsStates() #199

Merged
merged 11 commits into from
Oct 30, 2024
Merged

Conversation

callebtc
Copy link
Contributor

@callebtc callebtc commented Oct 29, 2024

Add checkProofsStates method to CashuWallet to get the spent enums from the mint, should retire checkProofsSpent which throws most information away.

I would prefer getting rid of checkProofsSpent entirely. If you agree, please delete it or let me know.

Edit: we're getting rid of checkProofsSpent.

@callebtc callebtc changed the title Wallet add check proofs states Add CashuWallet.checkProofsStates Oct 29, 2024
@callebtc callebtc changed the title Add CashuWallet.checkProofsStates Add CashuWallet.checkProofsStates() Oct 29, 2024
@Egge21M
Copy link
Collaborator

Egge21M commented Oct 29, 2024

Can we be sure that a mint will ALWAYS return the state in the same order as it received the proofs? This is not explicitly defined in NUT07 and if the mint does actually not do that, we need to take care of this in order to avoid lost nuts.

@callebtc
Copy link
Contributor Author

Can we be sure that a mint will ALWAYS return the state in the same order as it received the proofs? This is not explicitly defined in NUT07 and if the mint does actually not do that, we need to take care of this in order to avoid lost nuts.

Agreed, good point. Fixed in code and fixed in the spec: cashubtc/nuts#181

@Egge21M
Copy link
Collaborator

Egge21M commented Oct 30, 2024

Thanks for updating! What do you think about this instead?

async checkProofsStates(proofs: Array<Proof>): Promise<Array<ProofState>> {
	const enc = new TextEncoder();
	const Ys = proofs.map((p: Proof) => hashToCurve(enc.encode(p.secret)).toHex(true));
	// TODO: Replace this with a value from the info endpoint of the mint eventually
	const BATCH_SIZE = 100;
	const states: Array<ProofState> = [];
	for (let i = 0; i < Ys.length; i += BATCH_SIZE) {
		const YsSlice = Ys.slice(i, i + BATCH_SIZE);
		const { states: batchStates } = await this.mint.check({
			Ys: YsSlice
		});
		const stateMap: { [y: string]: ProofState } = {};
		batchStates.forEach((s) => {
			stateMap[s.Y] = s;
		});
		for (let j = 0; j < YsSlice.length; j++) {
			const state = stateMap[YsSlice[j]];
			if (!state) {
				throw new Error('Could not find state for proof with Y: ' + YsSlice[j]);
			}
		states.push(state);
		}
	}
	return states;
}

By introducing a map for the returned states, we can check whether a state is present in constant time. Before the worst case scenario was a quadratic + running time

@callebtc
Copy link
Contributor Author

By introducing a map for the returned states, we can check whether a state is present in constant time. Before the worst case scenario was a quadratic + running time

sure go ahead and push and merge it

@Egge21M Egge21M merged commit d4bbc0f into development Oct 30, 2024
4 checks passed
@Egge21M Egge21M deleted the wallet-add-checkProofsStates branch October 30, 2024 10:31
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.

2 participants