Update integration docs

[asalzmann]

We are adding PFM to Stride next week.

In v7.1.2, the integrations docs say

// Register packet forward middleware AppModule
app.moduleManager = module.NewManager(
    ...
    packetforward.NewAppModule(app.PacketForwardKeeper),
)

But a second argument is required by NewAppModule

// NewAppModule creates a new packetforward module
func NewAppModule(k *keeper.Keeper, ss exported.Subspace) AppModule {
	return AppModule{
		keeper:         k,
		legacySubspace: ss,
	}
}

What should legacySubspace be? Why is it required? @Reecepbcups

Related PR: #128

[Reecepbcups]

@asalzmann This is an SDK v45 -> 46+ store migration requirement (x/params is being removed in SDK v51, so have to migrate before)

Since your chain is already on SDK v47 and you are adding it, you can just set it to nil. Does not apply as there is no migration required

packetforward.NewAppModule(app.PacketForwardKeeper, nil),

Thanks for the callout on this, Ill modify the docs to add clarrification

[asalzmann]

Got it, I appreciate the quick response!

A few other discrepancies I noticed. From the integration docs

// Initialize the packet forward middleware Keeper
// It's important to note that the PFM Keeper must be initialized before the Transfer Keeper
app.PacketForwardKeeper = packetforwardkeeper.NewKeeper(
    appCodec,
    keys[packetforwardtypes.StoreKey],
    app.GetSubspace(packetforwardtypes.ModuleName),
    app.TransferKeeper, // will be zero-value here, reference is set later on with SetTransferKeeper.
    app.IBCKeeper.ChannelKeeper,
    appKeepers.DistrKeeper,
    app.BankKeeper,
    app.IBCKeeper.ChannelKeeper,
)

(1) app.GetSubspace in the integration docs but not the code
(2) authority is not in the integration docs, but is in the code

Code for reference

// NewKeeper creates a new forward Keeper instance
func NewKeeper(
	cdc codec.BinaryCodec,
	key storetypes.StoreKey,
	transferKeeper types.TransferKeeper,
	channelKeeper types.ChannelKeeper,
	distrKeeper types.DistributionKeeper,
	bankKeeper types.BankKeeper,
	ics4Wrapper porttypes.ICS4Wrapper,
	authority string,
) *Keeper {
	return &Keeper{
		cdc:            cdc,
		storeKey:       key,
		transferKeeper: transferKeeper,
		channelKeeper:  channelKeeper,
		distrKeeper:    distrKeeper,
		bankKeeper:     bankKeeper,
		ics4Wrapper:    ics4Wrapper,
		authority:      authority,
	}
}

[asalzmann]

@Reecepbcups can you weigh in on:

(1) app.GetSubspace in the integration docs but not the code
(2) authority is not in the integration docs, but is in the code

Should we ignore app.GetSubspace and use the x/gov authority?

[Reecepbcups]

@asalzmann

The following changes are pending review & merge here: #147 - this gives the correct documentation

1. The GetSubspace was removed from the Keeper as it is no longer used within the module (v7.1.x migrates from x/params -> params in the keeper). This was done in #140 since it is ONLY used in the Consensus 1->2 store migration (via packetforward.NewAppModule). SDK v0.51 removes x/params entirely, so we are migrated from it in the release of v7.1.X proactively.

Since stride is just now adding the PFM module, the subspace in packetforward.NewAppModule(app.PacketForwardKeeper, app.GetSubspace(packetforwardtypes.ModuleName)), will never be run since you come in on ConsensusVersion 2. You can use this exactly if you feel more comfortable as outlined in the updated docs PR above :)

2. Yes, you should use gov as the authority unless you have a DAO/multisig who will handle the params for the module