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: google cloud platform pubsub plugin integration #450

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Ikari
Copy link

@Ikari Ikari commented Jun 12, 2024

This PR have two changes:

  • Google's pubsub integration: hathor_pubsub.js is a plugin that allows the wallet to send external notifications to a pubsub topic
  • Fixing multisig param types: when deploying on some cloud environments (such as GCP Cloud Run), the multisig_seed${x}max_signatures and multisig_seed${x}num_signatures are threated as strings, not integers. When starting the multisig, the comparison made with this params is a exact on ( "===") so it fails. By parsing the params before we can avoid that.

src/controllers/index.controller.js Ln 100-101:

  const mconfig = config.multisig[multisigKey];
    console.log('mconfig', mconfig);
    if (!(mconfig
           && (mconfig.total && mconfig.numSignatures && mconfig.pubkeys)
           && (mconfig.pubkeys.length === mconfig.total)
           && (mconfig.numSignatures <= mconfig.total))) {
      // Missing multisig items
      console.error(`Improperly configured multisig for seed ${multisigKey}.`);
      res.send({
        success: false,
        message: `Improperly configured multisig for seed ${multisigKey}.`
      });
      return;

@Ikari Ikari requested a review from pedroferreira1 as a code owner June 12, 2024 01:01
@pedroferreira1 pedroferreira1 requested a review from r4mmer June 12, 2024 13:22
@pedroferreira1
Copy link
Member

@Ikari the PR is approved, it just needs a rebase so we can continue with the merge

@Ikari
Copy link
Author

Ikari commented Jun 22, 2024

@Ikari the PR is approved, it just needs a rebase so we can continue with the merge

Rebased!

@pedroferreira1
Copy link
Member

@Ikari we've fixed a bug in codecov, so we can make the CI succeed. Can you do one more rebase please, to update your branch, so we can check if the CI completes?

@Ikari
Copy link
Author

Ikari commented Jul 3, 2024

@pedroferreira1 sure thing, branch updated.

Copy link

codecov bot commented Jul 4, 2024

Codecov Report

Attention: Patch coverage is 0% with 12 lines in your changes missing coverage. Please review.

Project coverage is 89.04%. Comparing base (fb2faae) to head (00b780c).

Files Patch % Lines
src/plugins/hathor_pubsub.js 0.00% 9 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #450      +/-   ##
==========================================
- Coverage   89.53%   89.04%   -0.50%     
==========================================
  Files          51       52       +1     
  Lines        2151     2163      +12     
  Branches      406      409       +3     
==========================================
  Hits         1926     1926              
- Misses        208      217       +9     
- Partials       17       20       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@luislhl luislhl mentioned this pull request Aug 16, 2024
1 task
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