Cnight contract extrinsics#1602
Conversation
Signed-off-by: Justin Frevert <justinfrevert@gmail.com>
justinfrevert
changed the title
Signed-off-by: Justin Frevert <justinfrevert@gmail.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 03f5abf24f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| let bounded_policy_id: BoundedVec<u8, ConstU32<CNIGHT_POLICY_ID_LENGTH>> = policy_id | ||
| .try_into() | ||
| .map_err(|_| Error::<T>::CardanoIdentifierLengthExceeded)?; |
There was a problem hiding this comment.
When this root call is submitted with any policy id shorter than 28 bytes, the BoundedVec conversion succeeds because it enforces only the maximum length, but block authors later read it through CNightObservationApi and convert it into [u8; 28] in primitives/mainchain-follower/src/idp/cnight_observation.rs:125, causing inherent-data creation to fail with InvalidOnchainStateCNight("cnight_policy_id"). Since Cardano policy IDs are fixed-width, reject anything whose length is not exactly CNIGHT_POLICY_ID_LENGTH before writing storage.
Useful? React with 👍 / 👎.
| MainChainAuthTokenAssetName::<T>::set( | ||
| asset_name | ||
| .try_into() | ||
| .map_err(|_| Error::<T>::CardanoIdentifierLengthExceeded)?, |
There was a problem hiding this comment.
If this setter is used with arbitrary bytes that fit the 32-byte bound but are not UTF-8, the runtime API still returns them and the cNIGHT observation IDP converts them to String in primitives/mainchain-follower/src/idp/cnight_observation.rs:116-119; that makes block authors fail to build the required inherent after the storage update. Genesis accepts this field only as a validated string, so this extrinsic should enforce the same UTF-8/ASCII constraint before storing the value.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 21c400fc21
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| let bounded_asset_name: BoundedVec<u8, ConstU32<CARDANO_ASSET_NAME_MAX_LENGTH>> = | ||
| asset_name.try_into().map_err(|_| Error::<T>::CardanoIdentifierLengthExceeded)?; |
There was a problem hiding this comment.
When this root call is submitted with a cNIGHT asset name containing arbitrary non-UTF-8 bytes that still fit the 32-byte bound, the storage update succeeds, but block authors later read it through CNightObservationApi and convert it into the String field of CNightAddresses in primitives/mainchain-follower/src/idp/cnight_observation.rs, causing inherent-data creation to fail with InvalidOnchainStateCNight("cnight_asset_name"). Genesis validates this field as a string, so this setter should enforce the same UTF-8/ASCII constraint before storing the value.
Useful? React with 👍 / 👎.
The new set_cnight_identifier / set_auth_token_asset_name extrinsics added methods to the pallet WeightInfo trait, but the runtime-side weights impl (runtime/src/weights/pallet_cnight_observation.rs) only implemented process_tokens, so the runtime failed to compile (E0046). Add placeholder weights for both methods, mirroring the pallet's SubstrateWeight values. Also apply cargo fmt and add the missing change file. Signed-off-by: Justin Frevert <justinfrevert@gmail.com> Assisted-by: Claude:claude-opus-4-8
…night-identifiers Signed-off-by: Justin Frevert <justinfrevert@gmail.com> Assisted-by: Claude:claude-opus-4-8
A Cardano policy id is a blake2b-224 hash, i.e. exactly CNIGHT_POLICY_ID_LENGTH bytes. set_cnight_identifier previously only enforced the BoundedVec upper bound, so a shorter (malformed) policy id was silently accepted. Reject any policy id whose length is not exactly CNIGHT_POLICY_ID_LENGTH with a new InvalidCNightPolicyIdLength error. The oversized-input test now expects this error for an over-length policy id (the exact check runs before the BoundedVec conversion); add a test for the short-policy-id case. Signed-off-by: Justin Frevert <justinfrevert@gmail.com> Assisted-by: Claude:claude-opus-4-8
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8452c925d0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| /// This extrinsic needs Root origin. | ||
| #[pallet::call_index(3)] | ||
| #[pallet::weight((T::WeightInfo::set_cnight_identifier(), DispatchClass::Normal))] | ||
| pub fn set_cnight_identifier( |
There was a problem hiding this comment.
Adding this dispatchable changes the runtime metadata, but this commit does not update any checked-in metadata/static/midnight_metadata*.scale files (I checked the diff and the metadata directory). The repo instructions call out extrinsic signature changes as requiring +rebuild-metadata; until that artifact is regenerated, consumers that rely on the static metadata will not see or be able to construct/decode these new cNIGHT calls correctly.
Useful? React with 👍 / 👎.
2 participants
Overview
There's not clear ways to set cardano contracts for testing contract upgradeability. This adds extrinsics for this. The expected flow for upgrading contracts in this way is:
Addresses gap in #1561.
🗹 TODO before merging
📌 Submission Checklist
git commit -s) for the DCO🧪 Testing Evidence
Please describe any additional testing aside from CI:
🔱 Fork Strategy
Links