Fix #22140 - Add bech32 encoding/decoding #23466

W0nda · 2024-10-11T19:20:27Z

Mark this if you consider it ready to merge
I've added tests (optional)
I wrote some lines in the book (optional)

Description

Add bech32 encoding/decoding but I don't know if I should integrate that to rax2 or not...

trufae · 2024-10-11T20:39:45Z

Maybe makes more sense to be in rahash2

trufae · 2024-10-11T20:44:57Z

Fix indentation rules

dont preincrement
Dont else after return

trufae · 2024-10-11T20:46:23Z

Its probably better to have it as a crypto plugin for encoding. Also chk the linting issues to follow the indentation rules (space before tab) and having some tests would be great too.

good work!

trufae · 2024-10-12T16:28:52Z

Will be good to have the plugin , take a look at the crypto_base64.c . if you dont expose this api and use it directly in the plugin it wont need to be new-abi specific

W0nda · 2024-10-12T16:56:59Z

I'm implementing tests, next pushed commits will be:

tests
implementation of bech32 as plugin

W0nda · 2024-10-12T23:34:55Z

The crypto plugins implementation isn't valid at all. Sorry for having pushed too soon.
I'll correct that and push tomorrow or next week.

trufae · 2024-10-13T09:38:47Z

sounds good. there's no hurry. ill cut the release before merging this

trufae · 2024-10-17T10:05:39Z

github decided to broke all ubuntu-latest images, i fixed that on master, can you rebase your patches on top of master?

W0nda · 2024-10-21T13:10:49Z

The encoding code as been tested and works but I don't know how to integrate it into r2. Each time I try to build it fails.

(The implem in crypto_bech32.c isn't totally finished but I tried to understand how to compile it first)

trufae · 2024-10-21T13:30:23Z

The header file is not there. Maybe you forgot to commit it?

trufae · 2024-10-21T13:39:41Z

libr/include/r_crypto.h

@@ -9,6 +9,7 @@
 #include <r_hash.h>
 #include <r_lib.h>
 #include <r_crypto/r_sm4.h>
+#include <r_crypto/r_bech32.h>


this file doesnt exist

trufae · 2024-10-21T13:40:41Z

libr/crypto/p/crypto_bech32.h

+	BECH32_ENCODING_BECH32M
+} bech32_encoding;
+
+R_API int bech32_encode(char *output, const char *hrp, const uint8_t *data, size_t data_len, bech32_encoding enc);


we dont want to expose new APIs that will break the abi contract. Can you make this static for now? actually its only purpose is to be used by the plugin so it shouldnt be an issue

also public apis must start with r_ and if you really want to expose them you can wrap them under R2_USE_NEW_ABI

trufae · 2024-10-21T13:42:15Z

test/unit/test_bech32.c

+	return 0;
+}
+
+void test_crypto_bech32_encode (void) {


trufae · 2024-10-21T13:42:41Z

test/unit/test_bech32.c

+	size_t data_len;
+	int i;
+	for (i = 0; i < sizeof (valid_checksum_bech32) / sizeof (valid_checksum_bech32[0]); i++) {
+		bech32_decode (hrp, data, &data_len, valid_checksum_bech32[i]);


these tests use an api that shouldnt be made public to protect the abi contract. so you can:

use RCrypto api to test bech32 with its plugin instead

expose this API via R2_USE_NEW_ABI, and prefix it with r_crypto_bech32_...

trufae · 2024-10-21T13:43:13Z

test/unit/test_bech32.c

+	char hrp[84];
+	size_t data_len;
+	int i;
+	for (i = 0; i < sizeof (valid_checksum_bech32) / sizeof (valid_checksum_bech32[0]); i++) {


better move this sizeof() to a const int size = .. one line above

trufae

. if you need more help better use the chat

trufae · 2024-10-21T18:26:05Z

please commit the missing file, i can take over the pr and do the needed fixes if you want, but the error in the build log complains about a file which i guess you didnt' commited

W0nda · 2024-10-24T18:23:24Z

Missing file coming.
Sorry for that

trufae · 2024-10-25T07:28:42Z

Another missing file

trufae · 2024-10-25T09:35:44Z

just fixing it on a branch, but this code have quite many errors and warnings. ill fix them up in my pr.

p/crypto_bech32.c: In function ‘update’:
p/crypto_bech32.c:188:56: error: ‘enc’ undeclared (first use in this function)
  188 |                 bech32_encode (in_out, hrp, data, len, enc);
      |                                                        ^~~
p/crypto_bech32.c:188:56: note: each undeclared identifier is reported only once for each function it appears in
p/crypto_bech32.c:188:40: warning: passing argument 2 of ‘bech32_encode’ makes pointer from integer without a cast [-Wint-conversion]
  188 |                 bech32_encode (in_out, hrp, data, len, enc);
      |                                        ^~~
      |                                        |
      |                                        char
p/crypto_bech32.c:64:46: note: expected ‘const char *’ but argument is of type ‘char’
   64 | int bech32_encode (char *output, const char *hrp, const uint8_t *data, size_t data_len, bech32_encoding enc) {
      |                                  ~~~~~~~~~~~~^~~
p/crypto_bech32.c:188:45: warning: passing argument 3 of ‘bech32_encode’ makes pointer from integer without a cast [-Wint-conversion]
  188 |                 bech32_encode (in_out, hrp, data, len, enc);
      |                                             ^~~~
      |                                             |
      |                                             char
p/crypto_bech32.c:64:66: note: expected ‘const uint8_t *’ {aka ‘const unsigned char *’} but argument is of type ‘char’
   64 | int bech32_encode (char *output, const char *hrp, const uint8_t *data, size_t data_len, bech32_encoding enc) {
      |                                                   ~~~~~~~~~~~~~~~^~~~
p/crypto_bech32.c:188:51: warning: passing argument 4 of ‘bech32_encode’ makes integer from pointer without a cast [-Wint-conversion]
  188 |                 bech32_encode (in_out, hrp, data, len, enc);
      |                                                   ^~~
      |                                                   |
      |                                                   size_t * {aka long unsigned int *}
p/crypto_bech32.c:64:79: note: expected ‘size_t’ {aka ‘long unsigned int’} but argument is of type ‘size_t *’ {aka ‘long unsigned int *’}
   64 | int bech32_encode (char *output, const char *hrp, const uint8_t *data, size_t data_len, bech32_encoding enc) {
      |                                                                        ~~~~~~~^~~~~~~~
p/crypto_bech32.c:190:32: warning: passing argument 1 of ‘bech32_decode’ makes pointer from integer without a cast [-Wint-conversion]
  190 |                 bech32_decode (hrp, data, len, in_out);
      |                                ^~~
      |                                |
      |                                char
p/crypto_bech32.c:106:38: note: expected ‘char *’ but argument is of type ‘char’
  106 | bech32_encoding bech32_decode (char *hrp, uint8_t *data, size_t *data_len, const char *input) {
      |                                ~~~~~~^~~
p/crypto_bech32.c:190:37: warning: passing argument 2 of ‘bech32_decode’ makes pointer from integer without a cast [-Wint-conversion]
  190 |                 bech32_decode (hrp, data, len, in_out);
      |                                     ^~~~
      |                                     |
      |                                     char
p/crypto_bech32.c:106:52: note: expected ‘uint8_t *’ {aka ‘unsigned char *’} but argument is of type ‘char’
  106 | bech32_encoding bech32_decode (char *hrp, uint8_t *data, size_t *data_len, const char *input) {
      |                                           ~~~~~~~~~^~~~
p/crypto_bech32.c: In function ‘end’:
p/crypto_bech32.c:198:28: error: ‘hrp’ undeclared (first use in this function)
  198 |         return update (cj, hrp, data, data_len, in_out, enc);
      |                            ^~~
p/crypto_bech32.c:198:33: error: ‘data’ undeclared (first use in this function)
  198 |         return update (cj, hrp, data, data_len, in_out, enc);
      |                                 ^~~~
p/crypto_bech32.c:198:39: error: ‘data_len’ undeclared (first use in this function)
  198 |         return update (cj, hrp, data, data_len, in_out, enc);
      |                                       ^~~~~~~~
p/crypto_bech32.c:198:49: error: ‘in_out’ undeclared (first use in this function); did you mean ‘ino_t’?
  198 |         return update (cj, hrp, data, data_len, in_out, enc);
      |                                                 ^~~~~~
      |                                                 ino_t
p/crypto_bech32.c:198:57: error: ‘enc’ undeclared (first use in this function); did you mean ‘end’?
  198 |         return update (cj, hrp, data, data_len, in_out, enc);
      |                                                         ^~~
      |                                                         end
p/crypto_bech32.c:198:16: error: too many arguments to function ‘update’
  198 |         return update (cj, hrp, data, data_len, in_out, enc);
      |                ^~~~~~
p/crypto_bech32.c:184:13: note: declared here
  184 | static bool update(RCryptoJob *cj, const ut8 *buf, size_t *len) {
      |             ^~~~~~
p/crypto_bech32.c: At top level:
p/crypto_bech32.c:210:19: warning: initialization of ‘_Bool (*)(RCryptoJob *, const uint8_t *, int)’ {aka ‘_Bool (*)(struct r_crypto_job_t *, const unsigned char *, int)’} from incompatible pointer type ‘_Bool (*)(RCryptoJob *, const uint8_t *, size_t *)’ {aka ‘_Bool (*)(struct r_crypto_job_t *, const unsigned char *, long unsigned int *)’} [-Wincompatible-pointer-types]
  210 |         .update = update,
      |                   ^~~~~~
p/crypto_bech32.c:210:19: note: (near initialization for ‘r_crypto_plugin_bech32.update’)
p/crypto_bech32.c: In function ‘bech32_decode’:
p/crypto_bech32.c:169:1: warning: control reaches end of non-void function [-Wreturn-type]
  169 | }
      | ^
p/crypto_bech32.c: In function ‘end’:
p/crypto_bech32.c:199:1: warning: control reaches end of non-void function [-Wreturn-type]
  199 | }
      | ^

trufae · 2024-10-25T09:51:10Z

here you have: #23528

cherry pick the last commit, as its the only one that is different from your branch, but even if it compiles this code wont work because it have lots of inconsistent issues and im not sure how to use the bech32 algorithm, what's hrp, adn so on. please update your pr with my commit and fix the code

trufae

^

W0nda · 2024-10-30T18:20:12Z

I can't build without r_bech32.h (that's why I left it last commit).

trufae · 2024-10-30T18:59:45Z

if you cant build withhout the r_bech32.h is because its included or referenced from a .d somewhere.

W0nda · 2024-10-31T15:06:26Z

Apparently I can build without r_bech32.h ...

trufae · 2024-10-31T15:52:22Z

There are conflicts with master now, also this function can be removed because its unused p/crypto_bech32.c:45:12: error: ‘hrplength’ defined but not used [-Werror=unused-function] 45 | static int hrplength(const unsigned char *bech32_checksum, const unsigned char size) { | ^~~~~~~~~

W0nda · 2024-10-31T16:41:11Z

I tried to resolve the conflicts i hope it worked.

trufae · 2024-11-01T08:46:40Z

Loooks like building now! Good job!

W0nda closed this Oct 11, 2024

W0nda deleted the mybranch branch October 11, 2024 19:24

W0nda restored the mybranch branch October 11, 2024 19:30

W0nda reopened this Oct 11, 2024

W0nda force-pushed the mybranch branch from dd1d5a0 to 4f77e37 Compare October 12, 2024 01:23

W0nda force-pushed the mybranch branch from a69c641 to 6292141 Compare October 12, 2024 22:47

W0nda force-pushed the mybranch branch from 50efdbb to 681b06a Compare October 20, 2024 23:54

trufae reviewed Oct 21, 2024

View reviewed changes

test/unit/test_bech32.c Outdated

return 0;

}

void test_crypto_bech32_encode (void) {

Copy link

Collaborator

trufae Oct 21, 2024

Choose a reason for hiding this comment

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

static

trufae reviewed Oct 21, 2024

View reviewed changes

trufae requested changes Oct 21, 2024

View reviewed changes

trufae added this to the 5.9.8 - gigglebytes milestone Oct 21, 2024

W0nda force-pushed the mybranch branch from 681b06a to 1e32d75 Compare October 24, 2024 18:23

trufae requested changes Oct 30, 2024

View reviewed changes

W0nda and others added 19 commits October 31, 2024 17:38

Fix radareorg#22140 - Add bech32 encoding/decoding

ef2164b

Put bech32 in crypto + code style compliance

f658747

Remove bech32 code from util

226d68a

Add tests and implement bech32 as crypto plugin

70946b5

A bit more work on crypto plugin implem

fe1c614

Add bech32 to rahash2.c & r_crypto.h

64eeea7

Add r_crypto/r_bech32

54012fb

Minor change to test_bech32.c + add code to crypto_bech32.c

2449e14

Fix the build, but still the code makes no sense

d3b5920

bech32 unit test wont build

a7ebed4

Fix tests

729e960

Encoding/decoding should work, need to implement the output

2cae618

Made the changes requested

2ff5c00

Made the changes requested N°2

02ef2d0

Remove license from r_bech32.h

b402308

Change return value in bech32_final_constant

fea2a97

Remove redefinition problem

8cc4fa0

Requested changes made but can't build when removing r_bech32.h

4fe12a7

Remove r_bech32.h

095c0f7

W0nda force-pushed the mybranch branch from bbf1be3 to 095c0f7 Compare October 31, 2024 16:40

Remove useless hrplength() function

9683d78

trufae merged commit 58bacd2 into radareorg:master Nov 1, 2024
42 checks passed

W0nda deleted the mybranch branch November 1, 2024 14:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #22140 - Add bech32 encoding/decoding #23466

Fix #22140 - Add bech32 encoding/decoding #23466

W0nda commented Oct 11, 2024 •

edited

Loading

trufae commented Oct 11, 2024

trufae commented Oct 11, 2024

trufae commented Oct 11, 2024

trufae commented Oct 12, 2024

W0nda commented Oct 12, 2024 •

edited

Loading

W0nda commented Oct 12, 2024 •

edited

Loading

trufae commented Oct 13, 2024

trufae commented Oct 17, 2024

W0nda commented Oct 21, 2024

trufae commented Oct 21, 2024

trufae Oct 21, 2024

trufae Oct 21, 2024

trufae Oct 21, 2024

trufae Oct 21, 2024

trufae Oct 21, 2024

trufae Oct 21, 2024

trufae left a comment

trufae commented Oct 21, 2024

W0nda commented Oct 24, 2024

trufae commented Oct 25, 2024

trufae commented Oct 25, 2024

trufae commented Oct 25, 2024

trufae left a comment

W0nda commented Oct 30, 2024

trufae commented Oct 30, 2024

W0nda commented Oct 31, 2024

trufae commented Oct 31, 2024

W0nda commented Oct 31, 2024

trufae commented Nov 1, 2024

Fix #22140 - Add bech32 encoding/decoding #23466

Fix #22140 - Add bech32 encoding/decoding #23466

Conversation

W0nda commented Oct 11, 2024 • edited Loading

trufae commented Oct 11, 2024

trufae commented Oct 11, 2024

trufae commented Oct 11, 2024

trufae commented Oct 12, 2024

W0nda commented Oct 12, 2024 • edited Loading

W0nda commented Oct 12, 2024 • edited Loading

trufae commented Oct 13, 2024

trufae commented Oct 17, 2024

W0nda commented Oct 21, 2024

trufae commented Oct 21, 2024

trufae Oct 21, 2024

Choose a reason for hiding this comment

trufae Oct 21, 2024

Choose a reason for hiding this comment

trufae Oct 21, 2024

Choose a reason for hiding this comment

trufae Oct 21, 2024

Choose a reason for hiding this comment

trufae Oct 21, 2024

Choose a reason for hiding this comment

trufae Oct 21, 2024

Choose a reason for hiding this comment

trufae left a comment

Choose a reason for hiding this comment

trufae commented Oct 21, 2024

W0nda commented Oct 24, 2024

trufae commented Oct 25, 2024

trufae commented Oct 25, 2024

trufae commented Oct 25, 2024

trufae left a comment

Choose a reason for hiding this comment

W0nda commented Oct 30, 2024

trufae commented Oct 30, 2024

W0nda commented Oct 31, 2024

trufae commented Oct 31, 2024

W0nda commented Oct 31, 2024

trufae commented Nov 1, 2024

W0nda commented Oct 11, 2024 •

edited

Loading

W0nda commented Oct 12, 2024 •

edited

Loading

W0nda commented Oct 12, 2024 •

edited

Loading