Skip to content

Remove CreateSignedTransaction deadcode#2479

Open
l0r1s wants to merge 1 commit into
devnet-readyfrom
remove-create-signed-tx-deadcode
Open

Remove CreateSignedTransaction deadcode#2479
l0r1s wants to merge 1 commit into
devnet-readyfrom
remove-create-signed-tx-deadcode

Conversation

@l0r1s
Copy link
Copy Markdown
Collaborator

@l0r1s l0r1s commented Mar 2, 2026

Summary

  • Remove dead CreateSignedTransaction impl and trait bounds — drand only uses send_unsigned_transaction (unsigned extrinsics with a signed payload), so create_signed_transaction was never called
  • Replace deprecated CreateInherent/new_inherent with CreateBare/new_bare across all mock files and runtime

Details

pallet_drand's offchain worker submits pulses via signer.send_unsigned_transaction(), which creates bare extrinsics through CreateBare::create_bare(). The CreateSignedTransaction trait and its implementations (runtime + 6 test mocks) were satisfying a trait bound but never invoked at runtime. This PR removes that dead code (~160 lines) and relaxes pallet_drand::Config to only require CreateBare<Call<Self>> + SigningTypes.

@l0r1s l0r1s added the skip-cargo-audit This PR fails cargo audit but needs to be merged anyway label Mar 2, 2026
sam0x17
sam0x17 previously approved these changes Mar 2, 2026
@l0r1s l0r1s force-pushed the remove-create-signed-tx-deadcode branch from d1395a7 to b9c2830 Compare May 25, 2026 22:38
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 25, 2026

🛡️ AI Review — Skeptic (security review)

VERDICT: SAFE

BASELINE scrutiny: l0r1s has repo write permission, an established account with substantial prior contributions, no committer mismatch, and no Gittensor allowlist hit found; branch remove-create-signed-tx-deadcode targets devnet-ready.

Static-only Skeptic review of the prefetched PR context and patch found a narrow removal/conversion around pallet_drand offchain transaction construction. The diff removes CreateSignedTransaction implementations and switches remaining bare extrinsic construction from deprecated CreateInherent/new_inherent to CreateBare/new_bare; the affected drand path still submits via send_unsigned_transaction with signed payload validation in validate_unsigned. No .github/ai-review/*, .github/copilot-instructions.md, dependency, build-script, or lockfile changes are present.

Findings

No findings.

Conclusion

No malicious behavior or security vulnerability was found in this diff. The PR appears to remove dead signed-transaction scaffolding without weakening drand unsigned transaction validation.


🔍 AI Review — Auditor (domain review)

VERDICT: 👎

Gittensor: UNKNOWN by allowlists; author has write permission and substantial recent subtensor history, so reviewed as an established repo contributor without incentive adjustment.

Static review found one blocking compile-coverage miss. I attempted a targeted cargo test -p pallet-subtensor test_recycle_tao_cannot_cross_preserve_threshold_in_high_ed_runtime --no-run, but Rustup could not create temp files under read-only /home/runner/.rustup; the finding below is based on static trait-bound analysis.

Spec-version auto-fix was not applied: the devnet runtime-version query could not resolve dev.chain.opentensor.ai from this sandbox, so I could not determine whether the live-network check would require a bump.

Findings

Sev File Finding
HIGH pallets/drand/src/lib.rs:164 Update all drand mock runtimes to implement CreateBare inline

Conclusion

The dead-code removal is directionally correct, but the new pallet_drand::Config bound was not propagated to all existing drand mock runtimes. This should be fixed before merge because affected test/workspace builds are expected to fail.

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

Comment thread pallets/drand/src/lib.rs
pub trait Config:
CreateSignedTransaction<Call<Self>> + CreateInherent<Call<Self>> + frame_system::Config
{
pub trait Config: CreateBare<Call<Self>> + SigningTypes + frame_system::Config {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] Update all drand mock runtimes to implement CreateBare

pallet_drand::Config now requires CreateBare<Call<Self>>, but this PR only converts part of the repo. Existing pallet_drand::Config implementors in pallets/subtensor/src/tests/mock_high_ed.rs, precompiles/src/mock.rs, and eco-tests/src/mock.rs still implement CreateInherent/CreateSignedTransaction and do not implement CreateBare, so crates that compile those mocks will no longer satisfy this bound. Convert those remaining mocks the same way as the files in this PR: implement frame_system::offchain::CreateBare, use UncheckedExtrinsic::new_bare(...), and remove the now-unused CreateSignedTransaction implementations.

@github-actions
Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: SAFE Auditor: 👎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-cargo-audit This PR fails cargo audit but needs to be merged anyway

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants