Add partial ord for begin block (backport #1980) (#2036)

* Add partial ord for begin block (#1980)

* wip

* Add Sequence method for ensuring a sequence of orders happen

* Mark all modules with no begin block logic

* Finish adding Partial Ord for begin block

* Add back IBChost

* Address roman's comment of missed constraint

(cherry picked from commit 5f17b19)

# Conflicts:
#	app/modules.go

* Update modules.go

Co-authored-by: Dev Ojha <[email protected]>

app/app.go

@@ -225,7 +225,7 @@ func NewOsmosisApp(
 	// NOTE: capability module's beginblocker must come before any modules using capabilities (e.g. IBC)
 
 	// Tell the app's module manager how to set the order of BeginBlockers, which are run at the beginning of every block.
-	app.mm.SetOrderBeginBlockers(orderBeginBlockers()...)
+	app.mm.SetOrderBeginBlockers(orderBeginBlockers(app.mm.ModuleNames())...)
 
 	// Tell the app's module manager how to set the order of EndBlockers, which are run at the end of every block.
 	app.mm.SetOrderEndBlockers(OrderEndBlockers(app.mm.ModuleNames())...)

app/modules.go

@@ -143,43 +143,26 @@ func appModules(
 	}
 }
 
-// orderBeginBlockers Tell the app's module manager how to set the order of
-// BeginBlockers, which are run at the beginning of every block.
-func orderBeginBlockers() []string {
-	return []string{
-		// Upgrades should be run VERY first
-		upgradetypes.ModuleName,
-		// Note: epochs' begin should be "real" start of epochs, we keep epochs beginblock at the beginning
-		epochstypes.ModuleName,
-		capabilitytypes.ModuleName,
-		minttypes.ModuleName,
-		poolincentivestypes.ModuleName,
-		distrtypes.ModuleName,
-		slashingtypes.ModuleName,
-		evidencetypes.ModuleName,
-		stakingtypes.ModuleName,
-		ibchost.ModuleName,
-		ibctransfertypes.ModuleName,
-		icatypes.ModuleName,
-		authtypes.ModuleName,
-		banktypes.ModuleName,
-		govtypes.ModuleName,
-		crisistypes.ModuleName,
-		genutiltypes.ModuleName,
-		authz.ModuleName,
-		paramstypes.ModuleName,
-		vestingtypes.ModuleName,
-		gammtypes.ModuleName,
-		incentivestypes.ModuleName,
-		lockuptypes.ModuleName,
-		poolincentivestypes.ModuleName,
-		tokenfactorytypes.ModuleName,
-		// superfluid must come after distribution and epochs
-		superfluidtypes.ModuleName,
-		bech32ibctypes.ModuleName,
-		txfeestypes.ModuleName,
-		wasm.ModuleName,
-	}
+// orderBeginBlockers returns the order of BeginBlockers, by module name.
+func orderBeginBlockers(allModuleNames []string) []string {
+	ord := partialord.NewPartialOrdering(allModuleNames)
+	// Upgrades should be run VERY first
+	// Epochs is set to be next right now, this in principle could change to come later / be at the end.
+	// But would have to be a holistic change with other pipelines taken into account.
+	ord.FirstElements(upgradetypes.ModuleName, epochstypes.ModuleName, capabilitytypes.ModuleName)
+
+	// Staking ordering
+	// TODO: Perhaps this can be relaxed, left to future work to analyze.
+	ord.Sequence(distrtypes.ModuleName, slashingtypes.ModuleName, evidencetypes.ModuleName, stakingtypes.ModuleName)
+	// superfluid must come after distribution & epochs.
+	// TODO: we actually set it to come after staking, since thats what happened before, and want to minimize chance of break.
+	ord.After(superfluidtypes.ModuleName, stakingtypes.ModuleName)
+	// TODO: This can almost certainly be un-constrained, but we keep the constraint to match prior functionality.
+	// IBChost came after staking, before superfluid.
+	// TODO: Come back and delete this line after testing the base change.
+	ord.Sequence(stakingtypes.ModuleName, ibchost.ModuleName, superfluidtypes.ModuleName)
+	// every remaining module's begin block is a no-op.
+	return ord.TotalOrdering()
 }
 
 func OrderEndBlockers(allModuleNames []string) []string {

osmoutils/partialord/partialord.go

@@ -76,6 +76,18 @@ func (ord *PartialOrdering) LastElements(elems ...string) {
 	ord.lastSealed = true
 }
 
+// Sequence sets a sequence of ordering constraints.
+// So if were making an ordering over {A, B, C, D, E}, and elems provided is {D, B, A}
+// then we are guaranteed that the total ordering will have D comes before B comes before A.
+// (They're may be elements interspersed, e.g. {D, C, E, B, A} is a valid ordering)
+func (ord *PartialOrdering) Sequence(seq ...string) {
+	// We make every node in the sequence have a prior node
+	for i := 0; i < (len(seq) - 1); i++ {
+		err := ord.dag.AddEdge(seq[i], seq[i+1])
+		handleDAGErr(err)
+	}
+}
+
 // TotalOrdering returns a deterministically chosen total ordering that satisfies all specified
 // partial ordering constraints.
 //

osmoutils/partialord/partialord_test.go

@@ -54,3 +54,22 @@ func TestNonStandardAPIOrder(t *testing.T) {
 	expOrdering = []string{"A", "B", "C", "D", "E", "F", "G"}
 	require.Equal(t, expOrdering, ord.TotalOrdering())
 }
+
+// This test ad-hocly tests combination of multiple sequences, first elements, and an After
+// invokation.
+func TestSequence(t *testing.T) {
+	// This test uses direct ordering before First, and after Last
+	names := []string{"A", "B", "C", "D", "E", "F", "G"}
+	ord := partialord.NewPartialOrdering(names)
+	// Make B A C a sequence
+	ord.Sequence("B", "A", "C")
+	// Make A G E a sub-sequence
+	ord.Sequence("A", "G", "E")
+	// make first elements D B F
+	ord.FirstElements("D", "B", "F")
+	// make C come after E
+	ord.After("C", "G")
+
+	expOrdering := []string{"D", "B", "F", "A", "G", "C", "E"}
+	require.Equal(t, expOrdering, ord.TotalOrdering())
+}