Skip to content
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

Put SidechainParams into the Env effect #59

Open
jstolarek opened this issue Jul 9, 2024 · 1 comment
Open

Put SidechainParams into the Env effect #59

jstolarek opened this issue Jul 9, 2024 · 1 comment
Labels
refactoring Code cleanups and improvements

Comments

@jstolarek
Copy link
Contributor

jstolarek commented Jul 9, 2024

In the off-chain code SidechainParams are all over the place, being explicitly passed into most of the functions only to deserialize scripts somewhere at the bottom of the call stack. This needs to stop. We have the effects system to deal with such things. Importantly, input-output-hk/trustless-sidechain#832 introduces the Env effect, which stores a global read-only environment. This is where SidechainParams should go, and they should only be recovered from the environment when needed.


IOG Jira: https://input-output.atlassian.net/browse/ETCM-7873

@tgunnoe tgunnoe transferred this issue from another repository Jul 25, 2024
@jstolarek jstolarek added the refactoring Code cleanups and improvements label Jul 30, 2024
@jstolarek jstolarek assigned jstolarek and unassigned jstolarek Jul 30, 2024
@jstolarek
Copy link
Contributor Author

Implementation notes based on some prototyping:

  • Once SidechainParams is added to Env, emptyEnv no longer makes sense and needs to be removed.
  • Sidechain initialization needs to be modified:
    initSidechain 
       r.
      InitSidechainParams 
      Int 
      Run (APP + r)
        { transactionId  TransactionHash
        , initTransactionIds  Array TransactionHash
        , sidechainParams  SidechainParams
        , sidechainAddresses  SidechainAddresses
        }
    initSidechain (InitSidechainParams isp) version = do
      let sidechainParams = toSidechainParams isp
    Current definition has the APP effect, which includes access to environment (READER Env effect). This means that SidechainParams should already exist and thus generating them inside a function and returning them as part of result makes little sense.
  • Tests need to be rejiggered. Consider this:
    testScenario1  PlutipTest
    testScenario1 =
      Mote.Monad.test "Successful reserve initialization with ADA as reserve token"
        $ Test.PlutipTest.mkPlutipConfigTest initialDistribution
        $ \alice → withUnliftApp (Wallet.withKeyWallet alice) do
            pkh <- getOwnPaymentPubKeyHash
            Test.Utils.withSingleMultiSig (unwrap pkh) $ do
              sidechainParams ← dummyInitialiseSidechain pkh
    As described in the previous bullet, SidechainParams are currently generated during sidechain initialization and this needs to be amended. This means that withSingleMultiSig needs to be changed to withEnv and take the full environment instead of just governance wallet. The tricky bit is finding the genesis UTxO, which needs to be done at the same place as getOwnPaymentPubKeyHash.
  • There's some design space in how to refactor withSingleMultiSig to withEnv. Currently, withSingleMultiSig assumes 1-of-1 multisignature governance. That is convenient for most of the tests, but unsuitable for testing multi-signature governance. In a similar way, we might wish to fix all parameters of SidechainParams, except the genesis UTxO, which needs to be found dynamically. This will work for almost all tests, except those that test the threshold.
  • TrustlessSidechain.SidechainParams module is missing explicit export interface.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Code cleanups and improvements
Projects
None yet
Development

No branches or pull requests

1 participant