Skip to content

feat(ebe): added multisig signing API to EBE#19

Merged
alextse-bg merged 1 commit into
masterfrom
WP-4519
Jun 10, 2025
Merged

feat(ebe): added multisig signing API to EBE#19
alextse-bg merged 1 commit into
masterfrom
WP-4519

Conversation

@alextse-bg
Copy link
Copy Markdown
Contributor

Ticket: WP-4519

@alextse-bg alextse-bg force-pushed the WP-4519 branch 2 times, most recently from eab7e82 to 8aaee59 Compare June 9, 2025 14:34
@alextse-bg alextse-bg requested a review from pranavjain97 June 9, 2025 14:46
Comment on lines +12 to +20
if (!source || !pub) {
throw new Error('Source and public key are required for signing');
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

well txPrebuild is also required. But ideally we'll move all this validation to api-ts after mohammad's changes

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

added a check for prebuild

['bitgo', 'source', 'pub'].forEach((key) => delete req.body[key]);

// Sign the transaction using BitGo SDK
const coin = bitgo.coin(req.params.coin);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

you need to verify the transaction

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

added a verification for prebuild

// Sign the transaction using BitGo SDK
const coin = bitgo.coin(req.params.coin);
try {
return await coin.signTransaction({ ...req.body, prv });
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lets not blindly pass req.body to signTransaction. parse the prebuild, verify it and then send it

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

with prebuild being verified we can now pass it in directly

Copy link
Copy Markdown
Contributor

@pranavjain97 pranavjain97 left a comment

Choose a reason for hiding this comment

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

Approving this for now to unblock E2E dev testing, but as a follow up-

  1. Add integration tests
  2. Use api-ts once enabled for EBE

@alextse-bg alextse-bg merged commit bf8f0d4 into master Jun 10, 2025
3 checks passed
@alextse-bg alextse-bg deleted the WP-4519 branch June 10, 2025 16:44
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