Skip to content

feat(roller): add "retry proving" for FFI panic #17

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
Merged
merged 15 commits into from
Oct 21, 2022
Merged

feat(roller): add "retry proving" for FFI panic #17

merged 15 commits into from
Oct 21, 2022

Conversation

Lawliet-Chan
Copy link
Contributor

@Lawliet-Chan Lawliet-Chan commented Oct 8, 2022 •
edited by icemelon
Loading

@Lawliet-Chan
Copy link
Contributor Author

@HAOYUatHZ I think if we just only deal with proving panic, maybe we can use golang recover() instead of twice proving.

@0xmountaintop
Copy link
Contributor

0xmountaintop commented Oct 19, 2022 •
edited
Loading

@HAOYUatHZ I think if we just only deal with proving panic, maybe we can use golang recover() instead of twice proving.

Ok! great idea! then we don't need the logic of Times any more？

@Lawliet-Chan
Copy link
Contributor Author

Lawliet-Chan commented Oct 19, 2022 •
edited
Loading

@HAOYUatHZ I think if we just only deal with proving panic, maybe we can use golang recover() instead of twice proving.

Ok! great idea! then we don't need the logic of Times any more？

Yeah, I think so, we need Times for recognizing if roller panics, we can skip the prove. recover() can handle the panic and keep the thread running.
If you only want to prevent panic from terminating the thread, I think recover() is enough. https://www.jianshu.com/p/0cbc97bd33fb

@Lawliet-Chan Lawliet-Chan mentioned this pull request Oct 20, 2022
Closed
0xmountaintop
0xmountaintop reviewed Oct 20, 2022
View reviewed changes
@0xmountaintop 0xmountaintop changed the title twice prove feat(roller): add "retry proving" for FFI panic Oct 20, 2022
@0xmountaintop 0xmountaintop merged commit ef31ce3 into staging Oct 21, 2022
@0xmountaintop 0xmountaintop deleted the twice_prove branch October 21, 2022 07:28
ChuhanJin pushed a commit that referenced this pull request Mar 15, 2023
@colinlyguo @JJVincentJJ
# This is a combination of 19 commits.
c9c148c 
# This is the 1st commit message:

feat: add monitor metrics (#262)

Co-authored-by: colinlyguo <[email protected]>
Co-authored-by: maskpp <[email protected]>
Co-authored-by: HAOYUatHZ <[email protected]>
Co-authored-by: Péter Garamvölgyi <[email protected]>
# This is the commit message #2:

refactor(bridge): remove layer1 client in in layer1 relayer constructor (#274)

Co-authored-by: vincent <[email protected]>
Co-authored-by: HAOYUatHZ <[email protected]>
# This is the commit message #3:

fix: add gas multiplier (#275)


# This is the commit message #4:

feat(libzkp): use dylib instead of staticlib  (#266)

Co-authored-by: maskpp <[email protected]>
Co-authored-by: HAOYUatHZ <[email protected]>
Co-authored-by: colin <[email protected]>
Co-authored-by: colinlyguo <[email protected]>
# This is the commit message #5:

Revert "fix: add gas multiplier (#275)" (#279)


# This is the commit message #6:

build: add nightly-2022-12-10 rust-related builder image (#282)


# This is the commit message #7:

fix(bridge): compatible with DynamicFeeTxType not supported chain (#280)

Co-authored-by: colinlyguo <[email protected]>
# This is the commit message #8:

feat(contract): enable whitelist relayer (#272)

Co-authored-by: Péter Garamvölgyi <[email protected]>
Co-authored-by: HAOYUatHZ <[email protected]>
# This is the commit message #9:

perf(bridge): execute relayer loops independently (#258)

Co-authored-by: colin <[email protected]>
Co-authored-by: HAOYUatHZ <[email protected]>
# This is the commit message #10:

feat(bridge): confirm block based on "safe" and "finalized" tags (#265)


# This is the commit message #11:

feat: allow to override L2 deployment when address is provided (#293)


# This is the commit message #12:

feat(contracts): Add fee vault (#223)


# This is the commit message #13:

feat(confirmations): Upgrade confirm (#291)


# This is the commit message #14:

feat(coordinator): Enable set ws compression level. (#292)


# This is the commit message #15:

feat(roller&coordinator): upgrade lizkp to zkevm-0215 version (#281)

Co-authored-by: HAOYUatHZ <[email protected]>
Co-authored-by: HAOYUatHZ <[email protected]>
Co-authored-by: xinran chen <[email protected]>
Co-authored-by: Ubuntu <[email protected]>
# This is the commit message #16:

build: update version to `alpha-v1.0` (#301)


# This is the commit message #17:

feat: import genesis batch during startup (#299)

Co-authored-by: HAOYUatHZ <[email protected]>
# This is the commit message #18:

feat(contracts): new bridge contracts (#288)

Co-authored-by: Haichen Shen <[email protected]>
Co-authored-by: Thegaram <[email protected]>
Co-authored-by: colin <[email protected]>
# This is the commit message #19:

chore: upgrade l2geth dependency for trace type (#304)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
2 more reviewers

@silathdiir silathdiir silathdiir approved these changes

@0xmountaintop 0xmountaintop 0xmountaintop approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Lawliet-Chan @0xmountaintop @silathdiir