Skip to content

cryptonote_core: tx verif util to collect transp amt commitments - #11088

Open
j-berman wants to merge 1 commit into
monero-project:masterfrom
j-berman:transp-amt-coms
Open

cryptonote_core: tx verif util to collect transp amt commitments#11088
j-berman wants to merge 1 commit into
monero-project:masterfrom
j-berman:transp-amt-coms

Conversation

@j-berman

Copy link
Copy Markdown
Collaborator

With FCMP++, we'll need the transparent amount commitments for all txs in a block in 3 distinct places:

  1. Adding v2 coinbase outputs to the db.
  2. Adding any transparent amount outputs to the curve tree.
  3. Validating amount commitments for torsion (may not be strictly necessary for this).

Since we do those actions at separate call sites, rather than re-calculate transparent amount commitments at every call site, we collect them one time and avoid repeating the calculation.

With FCMP++, we'll need the transparent amount commitments for
all txs in a block in 3 distinct places:

1) Adding v2 coinbase outputs to the db.
2) Adding any transparent amount outputs to the curve tree.
3) Validating amount commitments for torsion (may not be strictly
necessary for this).

Since we do those actions at separate call sites, rather than
re-calculate transparent amount commitments at every call site,
we collect them one time and avoid repeating the calculation.

@j-berman j-berman left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Included links showing the benefit of the std::reference_wrapper (to avoid copies), and the purpose of the distinct params across the function definitions

Comment on lines +340 to +343
std::vector<std::reference_wrapper<const transaction>> collect_transparent_amount_commitments(
const transaction &miner_tx,
const std::vector<std::pair<transaction, blobdata>> &tx_pairs,
std::unordered_map<uint64_t, rct::key> &transparent_amount_commitments_inout)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Comment on lines +354 to +357
std::vector<std::reference_wrapper<const transaction>> collect_transparent_amount_commitments(
const transaction &miner_tx,
const std::vector<transaction> &txs,
std::unordered_map<uint64_t, rct::key> &transparent_amount_commitments_inout)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Comment on lines +368 to +370
std::vector<std::reference_wrapper<const transaction>> collect_transparent_amount_commitments(
const std::unordered_map<crypto::hash, std::pair<transaction, blobdata>> &txs_by_txid,
std::unordered_map<uint64_t, rct::key> &transparent_amount_commitments_inout)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@j-berman

Copy link
Copy Markdown
Collaborator Author

I removed the reference_wrapper returns over in seraphis-migration#460

I think we'll want that change applied in this PR as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants