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

chore(e2e/app): DRAFT deploy contracts to test SILK against #2356

Closed
wants to merge 3 commits into from

Conversation

Zodomo
Copy link
Contributor

@Zodomo Zodomo commented Nov 1, 2024

This is a draft PR seeking commentary on the Go I wrote. This is my first attempt at writing anything in Go, so I explicitly welcome criticism, feedback, and suggestions. Please let me know what you like, what you don't, and feel free to ask about why I did things certain ways if it is unclear.

issue: none

Comment on lines +81 to +88
switch i {
case 0:
deployment.WstETH = addr
case 1:
deployment.RETH = addr
case 2:
deployment.METH = addr
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: this is a bit fragile. Do we need TokenDeployment to be a struct? Can it be a simple map, and then we simply do deployment[token.name] = addr?

Copy link
Collaborator

Choose a reason for hiding this comment

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

indeed, or something like:

token := struct {
   setter func(common.Address)
   ..
}{
  {
    setter: func(addr common.Addrss) { deployment.WstETH = addr }
  }
  ....
}

token.SetAddress(addr)

deployment.METH = addr
}

log.Info(ctx, "Token deployed",
Copy link
Collaborator

Choose a reason for hiding this comment

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

lets make this debug, info should only be main workflow steps, this is a sub-step

}

func deploySymbioticVaults(ctx context.Context, def Definition, deployedTokens map[uint64]TokenDeployment) (map[uint64]SymbioticVaultDeployment, error) {
vaults := []string{
Copy link
Collaborator

Choose a reason for hiding this comment

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

suggest combining this and tokens above into a single package-level struct.

@corverroos corverroos changed the title chore(e2e): DRAFT deploy contracts to test SILK against chore(e2e/app): DRAFT deploy contracts to test SILK against Nov 5, 2024
@Zodomo Zodomo closed this Nov 8, 2024
@Zodomo Zodomo deleted the zodomo/silk-test-contracts branch November 8, 2024 00:11
@Zodomo
Copy link
Contributor Author

Zodomo commented Nov 8, 2024

closed as this was more or less me just experimenting with the go codebase

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.

3 participants