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

Add sender HMAC to MessageV2 #218

Merged
merged 29 commits into from
Feb 2, 2024
Merged

Add sender HMAC to MessageV2 #218

merged 29 commits into from
Feb 2, 2024

Conversation

nplasterer
Copy link
Contributor

@nplasterer nplasterer commented Jan 19, 2024

Closes #216

Follows the pattern put in place here: xmtp/xmtp-js#519

This fixes the iOS entitlement issue for push notifications by adding additional information to the payload. See: xmtp/XIPs#35

@nplasterer nplasterer self-assigned this Jan 19, 2024
kele-leanes and others added 14 commits January 30, 2024 12:02
* rename module from XMTP to XMTPiOS

* more renaming

* remove xmtp-rust-swift references

* Use new V2 client from libxmtp

* building clean

* remove PublishResponse

* Use actual dep instead of local

* Update XMTP.podspec

Co-authored-by: Naomi Plasterer <[email protected]>

* implement streaming and pagination

* Update XMTP.podspec

Co-authored-by: Cameron Voell <[email protected]>

* disable file_length linter

---------

Co-authored-by: Naomi Plasterer <[email protected]>
Co-authored-by: Cameron Voell <[email protected]>
* Access libxmtp-swift with static binaries

* Update libxmtp ref

* Updated reference to libxmtp-swift merge on main
* Add Key Material Function (#222)

* add ket material field to ios

* bump the podspec

* Update podspec LibXMTP ref

* Update libxmtp-swift refs

* Update libxmtp-swift ref

* Updated libxmtp-swift ref to tagged release

* Update XMTP.podspec version

---------

Co-authored-by: cameronvoell <[email protected]>
@nplasterer nplasterer marked this pull request as ready for review February 1, 2024 03:44
@nplasterer nplasterer requested a review from a team as a code owner February 1, 2024 03:44
@@ -9,6 +9,10 @@ import XCTest
@testable import XMTPiOS

struct NumberCodec: ContentCodec {
func shouldPush(content: Double) throws -> Bool {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would you mind adding a test in here that shows you can get the shouldPush method correctly from the decodedMessage kind of like we should have fallback tests. 🙏

Copy link
Contributor

Choose a reason for hiding this comment

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

seems that Xmtp_MessageContents_EncodedContent does not have the shouldPush method available which is where the fallback is coming from. Should I get it from there or from a different place?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was my bad from looking at JS it's on the messagev2 so wrote a test for that scenario and pushed it up. 1967934

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks! @nplasterer

Copy link
Contributor Author

@nplasterer nplasterer left a comment

Choose a reason for hiding this comment

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

Looks like you have two legit linter issues

/Users/runner/work/xmtp-ios/xmtp-ios/Sources/XMTPiOS/Messages/MessageV2.swift:105:91: error: Force Unwrapping Violation: Force unwrapping should be avoided

/Users/runner/work/xmtp-ios/xmtp-ios/Sources/XMTPiOS/ConversationV2.swift:249:50: error: Force Cast Violation: Force casts should be avoided (force_cast)

@nplasterer
Copy link
Contributor Author

I can't approve this since I opened it but would be good to get eyes from @rygine and or @Bren2010 on the cryptography parts and @nakajima on the iOS code 🙏 But looks good to me.

@rygine
Copy link

rygine commented Feb 1, 2024

the crypto code seems OK, but i'll defer to someone with more knowledge. all the other pieces look to be in the right places.

@nplasterer
Copy link
Contributor Author

@kele-leanes do you mind bumping the pod spec so we can consume this in RN?

@nplasterer nplasterer merged commit c6a93cd into main Feb 2, 2024
1 check passed
@nplasterer nplasterer deleted the np/ios-entitlement branch February 2, 2024 02:03
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.

Add sender HMAC to MessageV2
5 participants