Feature: allow overriding module address

[kopy-kat]

Problem

Currently, when using the installation helper for any module and the helpers that require the module address, the constant is used. This hinders experimentation with other modules, such as new features, so we should allow for overriding this.

Solution

Imo the easiest solution is for every function that requires the usage of the now constant module address, there should be an optional override. The other solution is to have some user-defined constant that would overwrite our constant. This would require less code changes and be easier to maintain but I'm not sure how exactly it could work

[Dhruv-Mishra]

By optional override do you mean having it implemented as an optional argument with a default value within all functions or overriding the constant itself, without changing the function definitions?

From what I know, to handle multiple constants or have runtime mutability for defined constants, context objects are passed within function or created at a global level to be used by functions. I have also come across adding these constants as a dependency which is provided at runtime.

I think we can also try exploring public static variables to be used within functions. If these variables are just being read by these functions as constants, then there should not be a problem about their mutability either.

[kopy-kat]

By optional override do you mean having it implemented as an optional argument with a default value within all functions or overriding the constant itself, without changing the function definitions?

adding an optional arg with default value to all funcs - this is a big effort but easiest to do conceptually

so in both of the cases above, we'd need some central config to handle these constants right?

we can also try exploring public static variables to be used within functions

this is interesting, but would it be possible to only selectively overwrite some of these or would all need to be overwritten?

[kopy-kat]

could you have a think about what you think the best change here would be - we are trying to optimize for:

1. as little changes to the client as possible (ideally existing client code does not need to be modified)
2. easiest devx when overwriting constants with the core case of only overwriting some module addresses and not all

I think it could be useful to have some code/pseudocode for changes to the normal consumer (if any) and how someone would overwrite these addresses

[Dhruv-Mishra]

I think the simplest yet best approach is to use a default context object that contains all the constant module addresses and have an optional context parameter within functions. For overriding the constants at the level of each function, devs can pass a partial object with only updates to the relevant constants. For changing the default constant itself, we can have a global configuration function. This would allows us to have a very granular control of which constants are used where, while also having the option to use global constants like we currently do.

A ts code demonstrating the same would look something like this:

//constants.ts file

const defaultContext = { //object with default value to all constants
    moduleA: "0x0",
    moduleB: "0x1",
    moduleC: "0x2",
}

let globalContext = { ...defaultContext } //global context to be used as the default context within functions
function setGlobalContext(overrides: Partial<typeof defaultContext>) { //setter for global context
    globalContext = { ...globalContext, ...overrides }
}

function getCurrentContext(functionLevelOverride?: Partial<typeof defaultContext>) { // function to return a context object with the overrides applied over global context
    return { ...globalContext, ...localOverrides }
}

----------------------------------------
// helper.ts file

// sample function using constants
function removeTransactionLimit( account: Account, contextOverride?: Partial<typeof defaultContext>) {
    const context = getCurrentContext(contextOverrides);
    account.add(context.moduleA)
    ...
    ...
}

---------------------------------------
// using it as a developer

const account: Account = {...}

// calling without overrides
performAction(account); 

// updating global context to override
setGlobalContext({ moduleA: "0x4" });
performAction(account); 

// function-level override
performAction(account, { moduleA: "0x5" });

If devs are not directly accessing the constants, and instead using our methods which use the constants internally, then there should be no changes at the developer side for their existing code.

[kopy-kat]

yeah this feels like the best approach, I'm not sure we need a per-function override though and either way we could add this down the line if it was needed

one downside is that people do use constants at the moment, they are exported eg like this - I think you're right that for those people this would be a breaking change but this feels unavoidable in any of the scalable solutions

imo it would be useful to start tackling this so lmk if you want to give this a shot

[Dhruv-Mishra]

Yes, a per function override wouldn’t make sense for a defined constant.
I can definitely take this up, do you want me to proceed with the context based approach?

[kopy-kat]

yeah that would be great thanks! on naming, the constants are all in the module/NAME/constant.ts files and on naming, they're currently all caps - we can either keep this or go with a normal caps approach since that could look better in the config object

[Dhruv-Mishra]

Regarding naming, I suggest we retain the existing constants for now, marking them as deprecated in this release to ensure backward compatibility. Simultaneously, we can introduce the context object and shift all internal methods to use the new context-based constants.

I will get started with this right away if it aligns with you.

[kopy-kat]

yes thats a great shout!

[Dhruv-Mishra]

To streamline the addition of constants, I initially grouped all constants from a module into a single object and used the spread operator (...) to include these module-specific objects in a globalConstant object. This approach allows new constants added to a module's file to automatically be included in globalConstant. However the spread operator does not execute until runtime. As a result, globalConstant can be undefined until explicitly initialized within the module.

// file1.ts
const CONSTANT_1 = '0x0'
export const MODULE_CONSTANTS = {CONSTANT_1}

// global.ts
import MODULE_CONSTANTS from './file1';

export const globalConstant = { ...MODULE_CONSTANTS }; //this might be undefined if accessed from outside the module

To avoid the initialization issue, we must directly use the value of the constants in the globalConstants object. This ensures proper initialization. This is the approach that I am proceeding with at the moment.
For future development within moduleSDK, this would mean that we have to add any new constants which we wish to export directly within the globalConstant object instead of adding it in the constants.ts file of every module. I was initially trying to avoid this, but this is the best way to ensure that constants remain accessible even before initialization.

An alternate approach is to use the singleton design pattern, but based on past developer feedback, I don't think they would like accessing the constants with a getter or factory functions, so I am skipping this.

[Dhruv-Mishra]

I have created a draft PR with the above concept applied to it:
#118

If it aligns with you, then I will extend it to the entire package.

[Dhruv-Mishra]

@kopy-kat is there anything pending in this issue? Should we close this?

[kopy-kat]

yep this is done thanks - implemented in #118