Skip to content
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.

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

RMN home and remote readers #137

Merged
merged 11 commits into from
Sep 19, 2024
Merged

RMN home and remote readers #137

merged 11 commits into from
Sep 19, 2024

Conversation

0xnogo
Copy link
Contributor

@0xnogo 0xnogo commented Sep 19, 2024

Introduction of an rmn home reader (will be modified with the new RMNHome.sol) and an rmn remote reader.

Also modelled what the data would look like in the offchain code (see types/config.go). This will replace the the /config.go in a later PR.

@0xnogo 0xnogo changed the title rmn home and remote readers RMN home and remote readers Sep 19, 2024
@0xnogo 0xnogo marked this pull request as ready for review September 19, 2024 08:25
@0xnogo 0xnogo requested a review from a team as a code owner September 19, 2024 08:25
commit/merkleroot/rmn/types/config.go Show resolved Hide resolved
commit/merkleroot/rmn/types/config.go Outdated Show resolved Hide resolved
commit/merkleroot/rmn/types/config.go Show resolved Hide resolved
commit/merkleroot/rmn/types/config.go Outdated Show resolved Hide resolved
commit/merkleroot/rmn/types/config.go Outdated Show resolved Hide resolved
internal/reader/rmn_home.go Show resolved Hide resolved
internal/reader/rmn_home.go Outdated Show resolved Hide resolved
internal/reader/rmn_home.go Outdated Show resolved Hide resolved
internal/reader/rmn_home.go Outdated Show resolved Hide resolved
internal/reader/rmn_home.go Outdated Show resolved Hide resolved
Copy link
Contributor

@dimkouv dimkouv left a comment

Choose a reason for hiding this comment

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

Overall looks good 🚀, please resolve the minor comments before merging.

And maybe ask another look for sync services.StateMachine, cause I am not familiar with this library and maybe I didn't spot any issues

commit/merkleroot/rmn/types/config.go Outdated Show resolved Hide resolved
commit/merkleroot/rmn/types/config.go Outdated Show resolved Hide resolved
commit/merkleroot/rmn/types/config.go Outdated Show resolved Hide resolved
}

// Node mirrors RMNHome.sol's Node struct
type Node struct {
Copy link
Collaborator

Choose a reason for hiding this comment

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

The structs will be exported in the gethwrappers but we don't have access to gethwrappers here so we have to do this duplication.

internal/reader/rmn_home.go Outdated Show resolved Hide resolved
internal/reader/rmn_home.go Outdated Show resolved Hide resolved
internal/reader/rmn_home.go Outdated Show resolved Hide resolved
internal/reader/rmn_home.go Show resolved Hide resolved
internal/reader/rmn_home.go Show resolved Hide resolved
commit/merkleroot/rmn/types/config.go Outdated Show resolved Hide resolved
}

// IsNodeObserver checks if a node is an observer for the given source chain
func IsNodeObserver(sourceChain SourceChain, nodeIndex int, totalNodes int) (bool, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This method is no doubt useful, but does it make more sense as a method on SourceChain maybe?

@0xnogo 0xnogo enabled auto-merge (squash) September 19, 2024 18:38
Copy link

Test Coverage

Branch Coverage
ng/rmn-readers 71.7%
main 70.7%

@0xnogo 0xnogo merged commit 824045e into main Sep 19, 2024
3 checks passed
@mateusz-sekara mateusz-sekara deleted the ng/rmn-readers branch November 8, 2024 09:22
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.

4 participants