-
Notifications
You must be signed in to change notification settings - Fork 94
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
Integrate Construction API 2.0 #120
Conversation
Pull Request Test Coverage Report for Build 4357
💛 - Coveralls |
6d4ebbb
to
bb9a8d9
Compare
734377a
to
723fb65
Compare
func (c *ConstructorHelper) AllAddresses(ctx context.Context) ([]string, error) { | ||
return c.keyStorage.GetAllAddresses(ctx) | ||
// BroadcastAll attempts to broadcast all ready transactions. | ||
func (c *CoordinatorHelper) BroadcastAll( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does this method handle partial failures, i.e. if one broadcast fails, does the subsequent one also fails or it will continue broadcasting?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The caller defines a BroadcastLimit
for the max number of times to attempt re-broadcast (https://github.com/coinbase/rosetta-cli/blob/04f379541ad59a15a9cbf0e8472b12cedbcc2fa7/configuration/configuration.go#L186-L188) and if broadcast failures should be ignored (https://github.com/coinbase/rosetta-cli/blob/04f379541ad59a15a9cbf0e8472b12cedbcc2fa7/configuration/configuration.go#L190-L192).
TL;DR however you want!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a question. But looks good 🔥
Related Issue: #112
Blocking PR: coinbase/mesh-sdk-go#118
Changes
construction
inconfiguration
rosetta-sdk-go/constructor