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

feat: serialize on the stack #75

Merged

Conversation

PastaPastaPasta
Copy link
Member

This builds on top of #74 to enhance library performance when integrated into other stuff

src/elements.cpp Show resolved Hide resolved
src/elements.cpp Show resolved Hide resolved
src/elements.cpp Show resolved Hide resolved
@github-actions
Copy link

'This PR has been flagged as stale due to no activity for over 60
days. It will not be automatically closed, but it has been given
a stale-pr label and should be manually reviewed.'

@PastaPastaPasta PastaPastaPasta requested review from knst and kwvg December 4, 2024 15:42
@UdjinM6 UdjinM6 removed the stale-pr label Dec 4, 2024
src/elements.cpp Outdated Show resolved Hide resolved
src/elements.cpp Outdated Show resolved Hide resolved
src/elements.cpp Outdated Show resolved Hide resolved
@PastaPastaPasta
Copy link
Member Author

All done

Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

utACK 30aa085

@PastaPastaPasta PastaPastaPasta changed the title Feat/serialize stack feat: serialize on the stack Dec 4, 2024
std::array<uint8_t, PrivateKey::PRIVATE_KEY_SIZE> PrivateKey::SerializeToArray(bool fLegacy) const
{
std::array<uint8_t, PRIVATE_KEY_SIZE> data{};
Serialize(data.data());
Copy link

Choose a reason for hiding this comment

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

is it forgotten to use fLegacy here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure, but I'm guessing no? both PrivateKey::Serialize(uint8_t *buffer) and PrivateKey::Serialize(const bool fLegacy) don't use it. Maybe. private keys are not affected by legacy / non legacy

Copy link

Choose a reason for hiding this comment

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

Yes, it's a dummy one here which is needed because our CBLSWrapper expects it to be that way for every ImplType.

Copy link

@knst knst left a comment

Choose a reason for hiding this comment

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

utACK 30aa085

Copy link
Collaborator

@kwvg kwvg left a comment

Choose a reason for hiding this comment

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

ACK 30aa085


Tested on develop (1e553102) by updating subtree, unit and functional tests passed (see branch test_bls_75).

Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

utACK 30aa085

@PastaPastaPasta PastaPastaPasta merged commit cc649f3 into dashpay:develop Dec 6, 2024
25 checks passed
@PastaPastaPasta PastaPastaPasta deleted the feat/serialize-stack branch December 6, 2024 05:39
@kwvg kwvg mentioned this pull request Dec 6, 2024
PastaPastaPasta added a commit to dashpay/dash that referenced this pull request Dec 17, 2024
…bb5c5b0 as efd5c56

3bbe16c build: stop tracking cmake dependency relic_conf.h.in (Kittywhiskers Van Gogh)
efd5c56 Squashed 'src/dashbls/' changes from 7e747e8a07..0bb5c5b032 (Kittywhiskers Van Gogh)
257fd5e revert: stop tracking cmake dependency relic_conf.h.in (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * Dependency for #6493
  * Expected subtree hash `7bec74f04710e6031590283cf405e3f141bc63310cafe5e70aae9b8d4c98cbef` (see [instructions](#6323 (review)) to calculate)
  * Includes [bls-signatures#75](dashpay/bls-signatures#75) and [bls-signatures#106](dashpay/bls-signatures#106)

  ## Breaking Changes

  None expected.

  ## Checklist:

  - [x] I have performed a self-review of my own code **(note: N/A)**
  - [x]  I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
  - [x] I have added or updated relevant unit/integration/functional/e2e tests **(note: N/A)**
  - [x] I have made corresponding changes to the documentation **(note: N/A)**
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  PastaPastaPasta:
    utACK 3bbe16c
  UdjinM6:
    subtree looks good, utACK 3bbe16c

Tree-SHA512: 3f6853f90dfe5e3040189742858b6728e4ab505513202216f1e2f7213569798d2f2e346d73ece7505f87dc2439fde4c3a51472461163fc7c21734a734cbc0bdb
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.

4 participants