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

Ridvan batch reveal cache #56

Merged
merged 12 commits into from
Mar 23, 2021
Merged

Conversation

rozaydin
Copy link

contains batch-reveal wallet code, tested locally, including timeout testing, local tests are failing (9 of them) but it's because our production-namebase have the same tests failing.

Signed-off-by: rozaydin <ridvan@namebase.io>
Signed-off-by: rozaydin <ridvan@namebase.io>
Signed-off-by: rozaydin <ridvan@namebase.io>
Signed-off-by: rozaydin <ridvan@namebase.io>
@rozaydin rozaydin self-assigned this Mar 19, 2021
const tx = await req.wallet.sendMTX(mtx, passphrase);
return res.json(200, {tx: tx.getJSON(this.network),
errors: errorMessages});
const revealCache = req.wallet.sendRevealResults;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you split this function up it's doing so many things so it's a bit hard to review

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed ...

Signed-off-by: rozaydin <ridvan@namebase.io>
Signed-off-by: rozaydin <ridvan@namebase.io>
Copy link

@turbomaze turbomaze left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the changes, could you address all the github actions complains about JSDoc stuff and trailing whitespaces? I think this is failing the eslint rules.

Additionally, can we remove the need to update namebase-hs-client? We can have two different hsd-proxy endpoints and communicate with them over http like normal, never hitting hs-client.


const revealCache = req.wallet.sendRevealResults;

const { cacheMisses: uniqueNames, cacheHits: processedReveals } = util

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice aliasing

lib/utils/util.js Outdated Show resolved Hide resolved
lib/wallet/wallet.js Outdated Show resolved Hide resolved
Signed-off-by: rozaydin <ridvan@namebase.io>
Signed-off-by: rozaydin <ridvan@namebase.io>
Copy link

@turbomaze turbomaze left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still reusing some variable names I'll update it

lib/utils/util.js Outdated Show resolved Hide resolved
@turbomaze turbomaze merged commit ed94d27 into production-namebase Mar 23, 2021
@turbomaze turbomaze deleted the ridvan-batch-reveal-cache branch March 23, 2021 08:51
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