Tokens ERC1155 Connector Support

[onelapahead]

• From [helm] Tokens ERC1155 Deployment firefly#372
• Does not have IT test for ERC1155 (yet) bc it requires a functional Ethconnect with the ERC1155 contract deployed, we'll save adding that for when we get Ethconnect deployed as well

[drewmarshburn]

Just curious, why is the ERC1155 connector enabled by default in the chart?

[onelapahead]

Mainly to match the ff CLI and the firefly E2E expectations but I have thought the same thing since I had to then disable it as part of the IT tests and the fact that it requires its own contract to be deployed.

Keeping it disabled by default for now is probably fine.

[drewmarshburn]

So I understand that the FF conf is built from the top-level config within the chart, but it does seem confusing to me that we have:

config:
  erc1155ContractAddress: 'blah'
erc1155:
  ....

My brain really wants:

erc1155:
  contractAddress: 'blah'

I'm thinking about the future when we have more connectors and ethconnect itself being deployed. Thoughts?

[onelapahead]

Great point / question. The ERC1155 contract address really is never used in the config templating helper for the tokens plugin: https://github.com/hfuss/firefly-helm-charts/blob/tokens-erc1155/charts/firefly/templates/_helpers.tpl#L239-L247

its purely config for ERC1155 not FireFly to your point and this will conflate config as we add more connectors.

And furthermore, the only bit of config we allow users to provide for the DataExchange as a supporting example is the API key which we did via dataexchange.apiKey. If the DataExchange was external so that dataexchange.enabled is false, there's config.dataexchangeAPIKey to provide the value to FireFly: https://github.com/hyperledger/firefly-helm-charts/blob/main/charts/firefly/templates/dataexchange/secret.yaml#L29-L31

So considering that, I agree. Will change the contract address and topic values to live under erc1155.