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.

Sign up for GitHub

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

Jump to bottom

feat: add support for IBC deposits to shielded #1163

Merged
merged 1 commit into from
Oct 22, 2024
Merged

feat: add support for IBC deposits to shielded #1163

merged 1 commit into from
Oct 22, 2024

Conversation

emccorson
Copy link
Contributor

@emccorson emccorson commented Oct 4, 2024 •
edited
Loading

This PR adds support for IBC deposits to shielded Namada addresses.

Testing instructions are the same as #1162 but running namadac shielded-sync is necessary before checking that balance was received.

Changed

  • Re-add source channel input element (since I think we will need to change channels relatively frequently before mainnet)
  • Rename IBCTransferParams to IbcTransferParams

Added

  • Support IBC deposits to Namada shielded addresses
  • Add SDK methods to get MASP address and to generate IBC shielding memo

Fixed

  • Fix Long value being passed instead of BigInt in timeoutTimestampNanoseconds
    • I don't think this is what was causing the timeouts over IBC but I'm not 100% sure

@emccorson emccorson force-pushed the ibc-shielded-integration branch 2 times, most recently from 95a7e54 to 3290f91 Compare October 20, 2024 11:15
@emccorson emccorson changed the title feat: add shielded support to ibc integration with transfer module feat: add support for IBC deposits to shielded Oct 20, 2024
@emccorson emccorson marked this pull request as ready for review October 20, 2024 12:11
jurevans
jurevans reviewed Oct 21, 2024
View reviewed changes
packages/sdk/src/tx/tx.ts Outdated Show resolved Hide resolved
jurevans
jurevans approved these changes Oct 21, 2024
View reviewed changes
Copy link
Collaborator

@jurevans jurevans left a comment

Choose a reason for hiding this comment

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

This looks really good! Only very minor comments

@emccorson emccorson force-pushed the ibc-shielded-integration branch from 3290f91 to c6081ab Compare October 22, 2024 05:56
@emccorson emccorson force-pushed the ibc-shielded-integration branch from e756260 to 50d8e61 Compare October 22, 2024 06:29
@emccorson emccorson merged commit 50d8e61 into main Oct 22, 2024
9 checks passed
@emccorson emccorson deleted the ibc-shielded-integration branch October 22, 2024 06:29
@github-actions github-actions bot temporarily deployed to production October 22, 2024 06:35 Inactive
mateuszjasiuk pushed a commit that referenced this pull request Oct 30, 2024
@emccorson @mateuszjasiuk
feat: add support for IBC deposits to shielded (#1163)
eb1c685
This was referenced Jan 13, 2025
Merged
Merged
Merged
Merged
Closed
Closed
Closed
Closed
Merged
Merged
Closed
Closed
This was referenced Jan 14, 2025
Open
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jurevans jurevans jurevans approved these changes

@euharrison euharrison Awaiting requested review from euharrison

@pedrorezende pedrorezende Awaiting requested review from pedrorezende

@mateuszjasiuk mateuszjasiuk Awaiting requested review from mateuszjasiuk

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@emccorson @jurevans