Closed evilrobot-01 closed 1 year ago
@oraclown as discussed yesterday, please could you kindly review the config items at https://github.com/tellor-io/tellor-pallet/blob/main/src/lib.rs#L96-L237 and see if there are any remaining that should be converted to constants?
@oraclown as discussed yesterday, please could you kindly review the config items at https://github.com/tellor-io/tellor-pallet/blob/main/src/lib.rs#L96-L237 and see if there are any remaining that should be converted to constants?
Reviewed them and here's some I think could be made constants, or that I have questions about:
GovernanceOrigin
should be a constant? Why would this change once initially set?MaxQueryDataLength
are we sure u32 is large enough? Query data can be quite large (e.g. if someone wants to put a giant API call on chain).MaxValueLength
are we sure u32 is large enough? People might want to put some very large exotic data on chain.RegisterOrigin
should be constant? Why would this change once set?StakingOrigin
same point as RegisterOrigin
Time
why would the on-chain time provider change once set?Xcm
this would only change if there's a major upgrade to XCM? I thought the point of switching to v3 was to move to something that will be used for awhile.XcmFeesAsset
If someone wanted to change this, it seems like a lot of pallet functionality would break (however briefly). It seems like we haven't accounted for this case enough to enable someone to change it, or am I mistaken?It might be best to think of these config items as soft constants, in that they are defined when first adding the pallet to the runtime, but values could be changed with a runtime upgrade. So in other words, contract constructor parameters where the contract could later be upgraded if required. Their values are compiled into the wasm binary rather than being stored in state, such as storage items.
Will group them by type:
Origins: GovernanceOrigin
, StakingOrigin
, RegisterOrigin
Wouldnt change after initially being set, just provides some flexibility in allowing the pallet to be configured for alternative origins based on use case. For example, the RegisterOrigin could be Root, or governance, a multisig etc. The Governance and Staking origins simply allow the runtime to map the source of the xcm message (e.g. contract address on evm parachain) into a local origin which can be validated. This was previously more fixed, so can always revert these two if you prefer?
Data Size: MaxQueryDataLength: u32
, MaxValueLength: u32
State transition proofs from parachains are limited by size as they need to be validated by the relay chain.
The Polkadot Host requires that the state transitions performed on parachains be specified as a Wasm executable. Proofs of new state transitions that occur on a parachain must be validated against the registered state transition function (STF) that is stored on the Relay Chain by the validators before Polkadot acknowledges a state transition has occurred on a parachain. The key constraint regarding the logic of a parachain is that it must be verifiable by the Relay Chain validators. Verification most commonly takes the form of a bundled proof of a state transition known as a Proof-of-Verification (PoV) block, which is submitted to the validators from one or more of the parachain collators to be checked. https://wiki.polkadot.network/docs/learn-parachains#definition-of-a-parachain
https://substrate.stackexchange.com/a/7030/3138 also contains some more practical info, noting that PoV size is capped to 5MiB. Parachains will therefore need to size this through configuration accordingly based on their needs.
Time:
The pallet is dependent on the current timestamp for submitted oracles, so this config item essentially just brings pallet_timestamp
into scope via the UnixTime
trait so we can get the current time. We could tightly couple to the actual pallet implementation, but loose coupling is preferred.
Xcm:
As with time above, this is just plugging the xcm subsystem into the pallet via a trait for sending xcm messages. The actual version of xcm used is encoded in the message (https://github.com/paritytech/xcm-format/#2-basic-top-level-format) and is based on the versions available within the actual xcm dependency in Cargo.toml
XcmFeesAsset: Each parachain may have different requirements for which asset they want to use for fee payment on the controller contract chain. They may be happy to use the native token, or they may bridge some of their own token across. This config item essentially enables flexibility to specify a token which best fits the needs of the parachain.
PS - it is possible to annotate storage items as unbounded
to have them excluded from the PoV, but that sounds less than ideal for an oracle use case where data validity is especially important.
Another limit is that of MAX_ETHEREUM_XCM_INPUT_SIZE
on the EthereumXcm pallet on Moonbase, which only comes into play when sending a dispute from parachain to gov contract. It seems like this is currently the ultimate constraining factor here.
/// Max. allowed size of 65_536 bytes.
pub(crate) const MAX_ETHEREUM_XCM_INPUT_SIZE: u32 = 2u32.pow(16);
Thanks for the detailed explanations, @evilrobot-01 ! Keep the GovernanceOrigin, StakingOrigin, RegisterOrigin as you have 'em.
Closing as no additional work required here at this time.
All config items derived from source smart contracts should be constants rather than configurable.