Skip to content

Conversation

ChuhanJin
Copy link
Contributor

No description provided.

Copy link
Contributor

@Lawliet-Chan Lawliet-Chan left a comment

Choose a reason for hiding this comment

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

roller/message/message.go should not be changed because it is copied from https://github.com/scroll-tech/scroll-node/blob/staging/rollers/message/message.go
And later we need to merge these two message.go into one

return c.Subscribe(ctx, "roller", traceChan, "register", authMsg)
}

func (c *Client) SubmitProof(ctx context.Context, proof *message.AuthZkProof) (bool, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why define this bool? And it is always false.

Copy link
Contributor

Choose a reason for hiding this comment

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

Returns true when successfully verified, otherwise returns false and an error message.

Copy link
Contributor

Choose a reason for hiding this comment

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

Now we have StatusProofError Should we need another error named StatusVerifyError here instead of 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.

@Lawliet-Chan I think keep bool is ok, and this will cause both side to change if we want to bring StatusProofError in, and the return value already has an error?

Copy link
Contributor

@Lawliet-Chan Lawliet-Chan Oct 19, 2022

Choose a reason for hiding this comment

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

cause both side to change

OK, fine. I thought that nil error can represent verify successfully just like the same codes in common-rs

@Lawliet-Chan
Copy link
Contributor

roller/message/message.go should not be changed because it is copied from https://github.com/scroll-tech/scroll-node/blob/staging/rollers/message/message.go And later we need to merge these two message.go into one

Fine, I see the new message.go

func (r *Roller) loadOrCreateKey() (*ecdsa.PrivateKey, error) {
keystoreFilePath := r.cfg.KeystorePath
if _, err := os.Stat(r.cfg.KeystorePath); os.IsNotExist(err) {
func loadOrCreateKey(cfg *config.Config) (*ecdsa.PrivateKey, error) {
Copy link
Contributor

@0xmountaintop 0xmountaintop Oct 13, 2022

Choose a reason for hiding this comment

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

maybe we can

func loadOrCreateKey(keystoreFilePath string, keystorePassword string) (*ecdsa.PrivateKey, error) {

?

0xmountaintop
0xmountaintop previously approved these changes Oct 13, 2022
Copy link
Contributor

@0xmountaintop 0xmountaintop left a comment

Choose a reason for hiding this comment

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

some nitpicks. Otherwise LGTM.

@ChuhanJin ChuhanJin closed this Oct 24, 2022
ChuhanJin pushed a commit that referenced this pull request Mar 15, 2023
# 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)
@OrestTa OrestTa deleted the new_ws_roller branch April 28, 2023 13:25
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.

5 participants