refactor: remove enc_publish functions and create separate encoding functions instead

[richard-ramos]

Description

Current c-bindings have encoding functions for both lightpush and relay that while at first view they appear to simplify the process of encoding messages using v1 encryption, in practice, they don't introduce benefits that make their existence worthy of the duplication of code and coupling lightpush and relay bindings to encoding functionality.

This PR refactors away this issue by removing the encryption functions for lightpush and relay, and instead create two functions to perform this operation, similar to how decoding is done.

Changes

• Delete waku_relay_publish_enc_asymmetric, waku_relay_publish_enc_symmetric, waku_lightpush_publish_enc_asymmetric and waku_lightpush_publish_enc_symmetric
• Add 2 new functions: waku_encode_symmetric and waku_encode_asymmetric whose output can be used directly in the publish functions of relay and lightpush
• Update c-bindings example
• Update the bindings README.md

[richard-ramos]

@Ivansete-status i tagged you as a reviewer since this PR would introduce a change in the c-bindings, and would like your blessings / thoughts about them. In the c-bindings example you can see how encryption is done with the proposed changes

[status-im-auto]

Jenkins Builds

❔	Commit	#️⃣	Finished (UTC)	Duration	Platform	Result
✔️	4e14073	#1	2023-08-09 16:15:12	~1 min	linux	📦deb
✔️	4e14073	#1	2023-08-09 16:15:51	~1 min	nix-flake	📄log
✔️	4e14073	#1	2023-08-09 16:16:36	~2 min	tests	📄log
✔️	4e14073	#1	2023-08-09 16:16:42	~2 min	tests	📄log
✔️	4e14073	#1	2023-08-09 16:17:40	~3 min	android	📦tgz
✔️	4e14073	#1	2023-08-09 16:18:10	~4 min	ios	📦tgz
✔️	4e14073	#2	2023-08-10 13:31:13	~15 sec	nix-flake	📄log
✔️	4e14073	#2	2023-08-10 13:32:08	~1 min	linux	📦deb
✔️	4e14073	#2	2023-08-10 13:33:38	~2 min	tests	📄log
✔️	4e14073	#2	2023-08-10 13:33:43	~2 min	tests	📄log
✔️	4e14073	#2	2023-08-10 13:34:59	~4 min	ios	📦tgz
✔️	4e14073	#2	2023-08-10 13:35:03	~4 min	android	📦tgz
✔️	dcd13f3	#3	2023-08-11 15:45:49	~33 sec	linux	📦deb
✔️	dcd13f3	#3	2023-08-11 15:46:59	~1 min	tests	📄log
✔️	dcd13f3	#3	2023-08-11 15:47:00	~1 min	tests	📄log
✔️	dcd13f3	#3	2023-08-11 15:47:11	~1 min	nix-flake	📄log
✔️	dcd13f3	#3	2023-08-11 15:48:52	~3 min	android	📦tgz
✔️	dcd13f3	#3	2023-08-11 15:50:14	~4 min	ios	📦tgz

[Ivansete-status]

Hey ! Thanks for inviting me to the party ! I like it very much !

Is a great API simplification where the encryption and signing are protocol-agnostic, and also the error cases are well considered within the library/encoding.go.

Not a big deal at all but maybe in the future we could also split the encryption and signing a bit more.

Thanks for that !

[Ivansete-status]

I wonder if we could return better feedback in case of an error

[richard-ramos]

Hm. I think it should be fine. the err variable will contain enough information to determine the cause of the error, i.e.:

• symmetric key is required
• couldn't decrypt using symmetric key
• private key is required
• couldn't decrypt using asymmetric key
• invalid message length
• non supported KeyKind
• unsupported wakumessage version

[Ivansete-status]

Likewise as per the previous comment

[chaitanyaprem]

Nice PR!

But I am wondering, this change would make it a 2 step API to the caller/user who want to send/receive encrypted messages. Wouldn't it be better to do encrypt/decrypt under the hood when the user invokes enc_publish(relay/lightpush)?

[richard-ramos]

@chaitanyaprem: But I am wondering, this change would make it a 2 step API to the caller/user who want to send/receive encrypted messages. Wouldn't it be better to do encrypt/decrypt under the hood when the user invokes enc_publish(relay/lightpush)?

Good question. I do have the following concerns if we follow that approach:

• the function interface is less elegant as you'd have to specify the pubsub topic, content topic, and timeout for publishing the message, besides the parameters required for the encryption.
• enc_publish would break the principle of single responsibility as it would deal with both encryption and publishing, and an error happening during the function execution would need to handle both publishing and encryption errors, while error handling with separate functions would arguably be easier to implement as you'd have a section for encryption errors, and a separate section for publishing errors.

[chaitanyaprem]

@chaitanyaprem: But I am wondering, this change would make it a 2 step API to the caller/user who want to send/receive encrypted messages. Wouldn't it be better to do encrypt/decrypt under the hood when the user invokes enc_publish(relay/lightpush)?

Good question. I do have the following concerns if we follow that approach:

* the function interface is less elegant as you'd have to specify the pubsub topic, content topic, and timeout for publishing the message, besides the parameters required for the encryption.

* `enc_publish` would break the principle of single responsibility as it would deal with both encryption and publishing, and an error happening during the function execution would need to handle both publishing and encryption errors, while error handling with separate functions would arguably be easier to implement as you'd have a section for encryption errors, and a separate section for publishing errors.

I do agree that it makes the interface less elegant, can we have them pass a struct for the encryption params and another struct for publish params? That would make this cleaner.
Since it is a cross-language API call, i am wondering if it is a good idea to have multiple calls across. Otherwise i agree with your single responsibility principle.

If we don't have a good solution for your concerns, then we can go ahead with this approach of splitting.

[chaitanyaprem]

LGTM apart from the comment.