-
Notifications
You must be signed in to change notification settings - Fork 3
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
Feat/batch #4
Feat/batch #4
Conversation
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.
LGTM with minor comments
data, err := strconv.ParseInt(string(v), 10, 64) | ||
if err != nil { | ||
// must not happen | ||
panic(err) |
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.
good to return error, but panic not bad in this case maybe.
} | ||
bs.PopBatchInfo() | ||
} | ||
// TODO: set da and key that match the current batch info |
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.
// TODO: set da and key that match the current batch info | |
// TODO: receive multiple DAs and use DA which is matched with the current batch info |
return nil | ||
} | ||
|
||
func (h *Host) updateBatchInfoHandler(args nodetypes.EventHandlerArgs) error { |
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.
seems not used yet
|
||
OutputSubmitterMnemonic: "", | ||
BridgeExecutorMnemonic: "", | ||
BatchSubmitterMnemonic: "", |
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.
Don't we need default configs for BatchConfig? and some validation too
@@ -51,6 +53,7 @@ func NewExecutor(cfg *executortypes.Config, db types.DB, sv *server.Server, logg | |||
db.WithPrefix([]byte(executortypes.ChildNodeName)), | |||
logger.Named(executortypes.ChildNodeName), cdc, txConfig, | |||
), | |||
batch: batch.NewBatchSubmitter(cfg.Version, cfg.DANodeConfig(), cfg.BatchConfig(), db.WithPrefix([]byte(executortypes.BatchNodeName)), logger.Named(executortypes.BatchNodeName), cdc, txConfig, homePath), | |||
|
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.
seems we are receving da node config separately, but want to use host as our da node?
or should we run separate da node?
@@ -72,8 +75,23 @@ func NewExecutor(cfg *executortypes.Config, db types.DB, sv *server.Server, logg | |||
zap.Duration("submission_interval", bridgeInfo.BridgeConfig.SubmissionInterval), | |||
) | |||
|
|||
executor.child.Initialize(executor.host, bridgeInfo) | |||
err = executor.host.Initialize(executor.child, int64(bridgeInfo.BridgeId)) | |||
da := executor.host |
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.
maybe future work?
No description provided.