migrate from eggroll to rollmelette #48

ZzzzHui · 2024-03-04T14:37:22Z

This is my first time writing Go. So feel free to point out any of my silly mistakes, when it's ready to review 😂

ZzzzHui · 2024-03-04T14:56:09Z

cli/cmd/send.go


-	result, err := client.WaitFor(ctx, inputIndex)


Hey @gligneul
Do you think something like waitForInput function is a good helper function to replace the WaitFor function above?

Similarly, to implement the stateRun function, seems this getNodeState function would be a good helper, but it's not exported.

guidanoli · 2024-03-06T20:15:26Z

One aspect that we could improve is the input codec situation.
I will do some experiments with Cap'n Proto, and how well we can encode inputs in the front-end (JavaScript) and in tests (Lua), and decode them in the back-end (Go).

ZzzzHui · 2024-03-11T11:27:51Z

cli/cmd/send.go

-	sendCmd.PersistentFlags().Uint32VarP(&sendArgs.accountIndex,
-		"account-index", "a", 0, "Forge account index when sending the transaction")
+	sendCmd.PersistentFlags().StringVarP(&sendArgs.fromAddress,
+		"from-address", "f", "0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266",
+		"Sender address when sending the transaction")


Note: instead of using account index, we now use address as argument directly

ZzzzHui · 2024-03-12T15:53:32Z

frontend/src/model/state.ts

-    BountyIndex: number;
-    Name: string;
-    ImgLink: string;
-    Exploit: string;


it was subtle to find this bug. Didn't realize that the case in interface fields needs to be the same as the JSON data fields, otherwise we only get undefined using .Exploit.
I will open another issue to format other interface fields to camelCase to comply with JSON recommendation

claudioantonio · 2024-03-14T21:20:07Z

Nice @ZzzzHui ! Tomorrow I will execute a complete test on this branch using the frontend and I will let you know about issues, if they exist, ok?

ZzzzHui · 2024-03-15T09:28:26Z

That would be great. Thank you Claudio !!

claudioantonio · 2024-03-18T23:48:05Z

I did not forget about the test, @ZzzzHui ! 😉

guidanoli · 2024-03-22T04:58:54Z

Please rebase this on top of main. 🙏

guidanoli · 2024-04-02T19:14:33Z

Given that @ZzzzHui is OOO, and this is a blocker to bump the Sunodo SDK to 0.3.0, I will continue the work to deliver this ASAP.

guidanoli · 2024-04-09T19:26:04Z

@claudioantonio Added the warning to the README.

ZzzzHui · 2024-04-16T08:13:14Z

README.md

@@ -92,6 +92,10 @@ make test

 ## CLI

+> [!WARNING]


I didn't experience something not working with the CLI itself

See my other comment about cast limiting argument size. This may actually be a limitation on the OS, which limits argument sizes by default? This is non-critical, so @claudioantonio and I agreed to have this broken for the time being, and fix it on a later PR. I think the way is to mimic how EggRoll was creating and signing transactions using go-ethereum.

Yes. It's a system issue. Try increase stack space limit using ulimit -s as mentioned here.

ZzzzHui · 2024-04-16T08:14:10Z

README.md

@@ -151,6 +155,10 @@ go run ./cli test \

 ## Populating DApp

+> [!WARNING]


The populate script doesn't work because it uses -a flag. I will replace with -f flag and everything seems to work

It also doesn't work because it is now using cast which limits the size of arguments to function calls. In our case, this limits the size of the input, so we can't send some bounties such as SQLite.

guidanoli · 2024-04-16T19:47:13Z

I've fixed some issues:

The front-end was still encoding inputs in the old way (4-byte selector specifying the input type concatenated with the serialized payload JSON object). I've updated it to use the current way (serialized JSON object containing kind string and payload JSON object).
Fixed inspect response processing (truncating first character of every line)
Fix small typo in "Sponsor bounty" form
Fix name of Go struct field

claudioantonio · 2024-04-18T13:15:23Z

Hi Gui! I finished my tests and everything passed!

Test create a bounty ✅
Test sponsor a bounty ✅
Test exploit with valid code ✅
Test exploit with invalid code ✅
Test submit a valid exploit code ✅
Test submit a invalid exploit code ✅
Test voucher generation when valid exploit code is submitted ✅
Test voucher execution for exploited bounty when proof is available ✅
Test voucher generation when bounty expires and sponsor request withdrawal ✅
Test voucher execution for expired bounty when proof is available ✅
Test no action buttons are available for not connected user ✅

ZzzzHui · 2024-04-19T04:02:09Z

Great! Thanks for the extensive tests. Do you also approve the merge of this PR? @claudioantonio
(EDIT: Just saw in the discord that you already think it's ready to merge. I will merge it now)

ZzzzHui · 2024-04-19T04:15:20Z

Thanks for the reviews, tests and help guys @guidanoli @claudioantonio

ZzzzHui self-assigned this Mar 4, 2024

ZzzzHui linked an issue Mar 4, 2024 that may be closed by this pull request

Move the framework from EggRoll to Rollmelette #28

Closed

ZzzzHui commented Mar 4, 2024

View reviewed changes

ZzzzHui force-pushed the feature/migrate-to-rollmelette branch 2 times, most recently from f862c06 to 81c3c5a Compare March 6, 2024 17:35

ZzzzHui force-pushed the feature/migrate-to-rollmelette branch from 81c3c5a to 8e78399 Compare March 11, 2024 11:13

ZzzzHui changed the title ~~[WIP] migrate from eggroll to rollmelette~~ migrate from eggroll to rollmelette Mar 11, 2024

ZzzzHui marked this pull request as ready for review March 11, 2024 11:14

ZzzzHui commented Mar 11, 2024

View reviewed changes

ZzzzHui commented Mar 12, 2024

View reviewed changes

ZzzzHui requested a review from a team March 12, 2024 15:54

ZzzzHui mentioned this pull request Mar 12, 2024

Comply with JSON naming recommendations #59

Closed

ZzzzHui force-pushed the feature/migrate-to-rollmelette branch from f78221b to bda9591 Compare March 22, 2024 10:38

guidanoli force-pushed the feature/migrate-to-rollmelette branch from bda9591 to e628a69 Compare April 2, 2024 20:00

guidanoli mentioned this pull request Apr 10, 2024

Bump sunodo/sdk to 0.4.0 #71

Closed

ZzzzHui mentioned this pull request Apr 16, 2024

Fix populate script #75

Closed

ZzzzHui commented Apr 16, 2024

View reviewed changes

ZzzzHui force-pushed the feature/migrate-to-rollmelette branch from eeba1d6 to acff1b7 Compare April 16, 2024 08:34

ZzzzHui linked an issue Apr 16, 2024 that may be closed by this pull request

Fix populate script #75

Closed

ZzzzHui and others added 4 commits April 16, 2024 16:31

migrate from eggroll to rollmelette

db35454

fix frontend for the migration

dfd9a03

Use input kind in-line

1898dbd

fix populate script

f55b913

guidanoli added 5 commits April 16, 2024 16:31

Add pnpm-lock.yaml to .prettierignore

09e3995

Fix inspect response processing

f52c3d1

Fix typo in "Sponsor bounty" page

8cd505a

Fix JSON name of Go struct field

de5c9e4

Fix advance request encoding

c2c7a44

guidanoli force-pushed the feature/migrate-to-rollmelette branch from acff1b7 to c2c7a44 Compare April 16, 2024 19:31

Fix back-end debug logs

4c361a0

guidanoli linked an issue Apr 17, 2024 that may be closed by this pull request

Bad key provided to slog #78

Closed

guidanoli mentioned this pull request Apr 17, 2024

Write Typescript CLI #79

Open

guidanoli approved these changes Apr 18, 2024

View reviewed changes

guidanoli requested a review from claudioantonio April 18, 2024 14:10

ZzzzHui merged commit 1a97a12 into main Apr 19, 2024

guidanoli deleted the feature/migrate-to-rollmelette branch April 19, 2024 12:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

migrate from eggroll to rollmelette #48

migrate from eggroll to rollmelette #48

ZzzzHui commented Mar 4, 2024

ZzzzHui Mar 4, 2024

guidanoli commented Mar 6, 2024

ZzzzHui Mar 11, 2024

ZzzzHui Mar 12, 2024

claudioantonio commented Mar 14, 2024 •

edited

Loading

ZzzzHui commented Mar 15, 2024

claudioantonio commented Mar 18, 2024

guidanoli commented Mar 22, 2024

guidanoli commented Apr 2, 2024

guidanoli commented Apr 9, 2024

ZzzzHui Apr 16, 2024

guidanoli Apr 16, 2024

ZzzzHui Apr 17, 2024

ZzzzHui Apr 16, 2024

guidanoli Apr 16, 2024

guidanoli commented Apr 16, 2024

claudioantonio commented Apr 18, 2024

ZzzzHui commented Apr 19, 2024 •

edited

Loading

ZzzzHui commented Apr 19, 2024

		@@ -151,6 +155,10 @@ go run ./cli test \

		## Populating DApp

		> [!WARNING]

migrate from eggroll to rollmelette #48

migrate from eggroll to rollmelette #48

Conversation

ZzzzHui commented Mar 4, 2024

Choose a reason for hiding this comment

guidanoli commented Mar 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

claudioantonio commented Mar 14, 2024 • edited Loading

ZzzzHui commented Mar 15, 2024

claudioantonio commented Mar 18, 2024

guidanoli commented Mar 22, 2024

guidanoli commented Apr 2, 2024

guidanoli commented Apr 9, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guidanoli commented Apr 16, 2024

claudioantonio commented Apr 18, 2024

ZzzzHui commented Apr 19, 2024 • edited Loading

ZzzzHui commented Apr 19, 2024

claudioantonio commented Mar 14, 2024 •

edited

Loading

ZzzzHui commented Apr 19, 2024 •

edited

Loading