-
Notifications
You must be signed in to change notification settings - Fork 36
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: Implement fraudproof generation mode in cosmos-sdk #250
feat: Implement fraudproof generation mode in cosmos-sdk #250
Conversation
1f37db3
to
8bdd5ae
Compare
82f1017
to
0391f77
Compare
baseapp/baseapp.go
Outdated
// make list of options to pass by parsing fraudproof | ||
app := NewBaseApp(appName, logger, db, txDecoder, options...) | ||
|
||
app.router = router |
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.
what’s the best way to reuse a route from a one app B1 to another app B2? Just passing in the same router from B1 doesn’t seem to work because during the route it uses B1 but I want it to use B2.
baseapp/baseapp.go
Outdated
// make list of options to pass by parsing fraudproof | ||
app := NewBaseApp(appName, logger, db, txDecoder, options...) | ||
|
||
app.router = router |
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.
what’s the best way to reuse a route from a one app B1 to another app B2? Just passing in the same router from B1 doesn’t seem to work because during the route it uses B1 but I want it to use B2.
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.
not 100% sure of the context here, mind elaborating or pointing me to something?
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.
when creating fraud proofs, we're creating an entirely new app, correct? so why do we need to pass the same router?
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.
Yes, we're creating an entirely new app but functionally we'd want the same routing method.
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.
sorry, I'm out of the loop, isn't the router stateless? what's the reason we need the same exact one?
also, in most cases we'll be working with the msgServiceRouter, correct?
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.
or I guess we'll need both if we want to support legacy stuff
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.
sorry I should clarify more, yes the router is stateless. But we'd still like the same function as routes to be used in the new baseapp. No state just logically.
Yes in most cases we'll be using msgServiceRouter
, is that also something I should initialize using the previous app's msgServiceRouter
?
41c96f1
to
a746788
Compare
8500a19
to
ce43a38
Compare
…d-fraudproof-generation-mode
baseapp/baseapp.go
Outdated
options = append(options, SetTracerFor(storeKey.Name(), storeKeyToSubstoreTraceBuf[storeKey.Name()])) | ||
options = append(options, AppOptionFunc(routerOpts[storeKey.Name()])) | ||
} | ||
newApp, err := SetupBaseAppFromParams(app.name+"WithTracing", app.logger, dbm.NewMemDB(), app.txDecoder, storeKeyNames, app.LastBlockHeight()+1, storeToLoadFrom, options...) |
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.
I would prefer a separate variable like newBlockHeight = app.LastBlockHeight()+1
at the top of this method and then using it here.
@@ -32,7 +32,7 @@ func SetSubstoreTracer(w io.Writer) StoreOption { | |||
|
|||
// SetTracerFor provides a BaseApp option function that sets the | |||
// tracer for a substore with given skey inside a multistore. | |||
func SetTracerFor(skey storetypes.StoreKey, w io.Writer) StoreOption { | |||
func SetTracerFor(skey string, w io.Writer) StoreOption { | |||
return func(cfg *multi.StoreParams, _ uint64) 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.
Why change from storetypes.StoreKey
to string
?
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.
For a StoreKey
, it is newly created in SetupBaseAppFromParams
using NewKVStoreKey
. Since SetTracerFor
is called before that, we don't have access to that newly created StoreKey
which is why we just simplify it to a string
. If I were to use StoreKey
of the previous app, even though it would have the same underlying name, the object itself would be different so string
is a better shared object than StoreKey
.
everything is moved to cosmos-sdk-rollmint repo |
Description
Before: When an optimint full node finds a fraudulent block, it tells cosmos-sdk to go into
fraudproof generation mode
.After: optimint can supply cosmos-sdk with a fraudulent block and tell it to actually generate a fraudproof.
In the
fraudproof generation mode
, a cosmos-sdk app reverts its state to a previous state before the fraudulent block was applied and enables tracing for modules specified by optimint.Parts covered from the cycle of a fraudproof:
Builds on top of #256
Closes: #249
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!
to the type prefix if API or client breaking changeCHANGELOG.md
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!
in the type prefix if API or client breaking change