Skip to content

fix(ci): correct git safe.directory path in benchmark weights workflow#1091

Merged
sameh-farouk merged 1 commit into
developmentfrom
fix/ci-benchmark-weights-safe-directory
Jun 1, 2026
Merged

fix(ci): correct git safe.directory path in benchmark weights workflow#1091
sameh-farouk merged 1 commit into
developmentfrom
fix/ci-benchmark-weights-safe-directory

Conversation

@sameh-farouk
Copy link
Copy Markdown
Member

Problem

The Generate benchmark weights workflow fails at the Git config step:

fatal: detected dubious ownership in repository at '/__w/ledger_chain/ledger_chain'
Error: Process completed with exit code 128.

After the repository rename (tfchainledger_chain), the runner checks the repo out at /__w/ledger_chain/ledger_chain, but 060_generate_benchmark_weights.yml hardcoded the safe.directory exception to the old path /__w/tfchain/tfchain. The exception no longer matches the actual checkout path, so git refuses to operate on the repo.

The benchmark step that runs before this (./target/production/tfchain benchmark ...) succeeds, so the binary name is unaffected — this safe.directory line is the only break.

Fix

Use $GITHUB_WORKSPACE (set by the runner to the real checkout path regardless of repo name) instead of the hardcoded path:

-          git config --global --add safe.directory /__w/tfchain/tfchain
+          git config --global --add safe.directory "$GITHUB_WORKSPACE"

Rename-proof; no other workflow hardcodes the old path (grep-verified).

🤖 Generated with Claude Code

…flow

The repository rename (tfchain -> ledger_chain) changed the runner checkout
path to /__w/ledger_chain/ledger_chain, but the safe.directory exception was
hardcoded to /__w/tfchain/tfchain, so git still rejected the repo with
"detected dubious ownership" (exit 128) at the Git config step.

Use $GITHUB_WORKSPACE so the exception always matches the real checkout path,
regardless of repository name.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@sameh-farouk sameh-farouk requested a review from LeeSmet as a code owner June 1, 2026 14:44
@sameh-farouk sameh-farouk merged commit 014469f into development Jun 1, 2026
1 check failed
sameh-farouk added a commit that referenced this pull request Jun 1, 2026
…flow (#1091)

The repository rename (tfchain -> ledger_chain) changed the runner checkout
path to /__w/ledger_chain/ledger_chain, but the safe.directory exception was
hardcoded to /__w/tfchain/tfchain, so git still rejected the repo with
"detected dubious ownership" (exit 128) at the Git config step.

Use $GITHUB_WORKSPACE so the exception always matches the real checkout path,
regardless of repository name.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
sameh-farouk added a commit that referenced this pull request Jun 1, 2026
…flow (#1091)

The repository rename (tfchain -> ledger_chain) changed the runner checkout
path to /__w/ledger_chain/ledger_chain, but the safe.directory exception was
hardcoded to /__w/tfchain/tfchain, so git still rejected the repo with
"detected dubious ownership" (exit 128) at the Git config step.

Use $GITHUB_WORKSPACE so the exception always matches the real checkout path,
regardless of repository name.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
sameh-farouk added a commit that referenced this pull request Jun 1, 2026
…flow (#1091)

The repository rename (tfchain -> ledger_chain) changed the runner checkout
path to /__w/ledger_chain/ledger_chain, but the safe.directory exception was
hardcoded to /__w/tfchain/tfchain, so git still rejected the repo with
"detected dubious ownership" (exit 128) at the Git config step.

Use $GITHUB_WORKSPACE so the exception always matches the real checkout path,
regardless of repository name.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
sameh-farouk added a commit that referenced this pull request Jun 1, 2026
…ry (#1081)

* fix(bridge): reset expiry timer on first signature after burn/refund expiry

After on_finalize expires a burn or refund transaction, re-proposals add
signatures via add_stellar_sig_burn_transaction / add_stellar_sig_refund_transaction
but tx.block was never updated. This caused on_finalize to immediately
re-expire the transaction in the same block the new signature was added
(since current_block - old_block >= RetryInterval was still true).

Also removes dead code in propose_stellar_burn_transaction_or_add_sig where
a redundant contains_key check made the block-update branch unreachable.

Closes #1080

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* chore: update benchmark `weights.rs` file for all pallets

* test(bridge): add regression tests for expiry block reset (#1080)

Cover the scenario the existing suite missed: a first signature added when
current_block - tx.block has again reached RetryInterval must reset tx.block
so on_finalize (which runs after extrinsics in the same block) does not wipe
the signature immediately.

Both tests fail without the fix (tx.block is not reset) and pass with it:
- burn_signature_after_expiry_resets_block_and_survives
- refund_signature_after_expiry_resets_block_and_survives

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(ci): use $GITHUB_WORKSPACE for git safe.directory in weights workflow (#1091)

The repository rename (tfchain -> ledger_chain) changed the runner checkout
path to /__w/ledger_chain/ledger_chain, but the safe.directory exception was
hardcoded to /__w/tfchain/tfchain, so git still rejected the repo with
"detected dubious ownership" (exit 128) at the Git config step.

Use $GITHUB_WORKSPACE so the exception always matches the real checkout path,
regardless of repository name.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
sameh-farouk added a commit that referenced this pull request Jun 1, 2026
…ts (#1087)

* fix(pallet-tfgrid): enforce node_certification in farming policy limits

## Problem

The FarmingPolicyLimit.node_certification field was never enforced in
get_farming_policy(). Per the spec (docs/misc/minimal_DAO.md):

> Certification. If set, only certified nodes can get this policy.
> Non certified nodes get a default policy.

A Diy node on a farm with node_certification=true in its limits would
receive the certified-only policy instead of falling back to a default.

On mainnet, farm 2995 (GoldAndGreen) has 4 Diy nodes (2 currently
online) earning certified-level farming rewards on policy 3.

## Root Cause

Two issues:

1. No certification check in the FarmingPolicyLimit path of
   get_farming_policy() — limits only checked end, cu, su, node_count.

2. get_default_farming_policy() (called when limits are exhausted)
   ignored node/farm certification entirely — it just picked the
   highest-ranked default policy regardless of the node's actual
   certification. Per the spec, default policy selection should follow:
   - First: highest farm cert + certified nodes
   - Then: highest farm cert + non-certified nodes
   - Then: no farm cert + certified nodes
   - Last: no certification at all

## Fix

- Add node_certification check at the start of the limits path: Diy
  nodes on certified-only farms fall back to a matching default.

- Replace get_default_farming_policy() with get_default_farming_policy_for()
  that filters by both node_certification and farm_certification. This
  affects all 4 existing fallback paths (end expired, cu exceeded, su
  exceeded, node_count exhausted) plus the new certification check.

## Tests

- diy_node_gets_default_policy_when_limit_requires_certification:
  Diy node on Gold farm with node_certification=true gets policy 1
  (Diy+Gold default) instead of policy 3 (Certified+Gold limited)

- certified_node_gets_limited_policy_when_limit_requires_certification:
  Certified node correctly receives the limited policy

- diy_node_gets_limited_policy_when_no_certification_required:
  Diy node gets limited policy when node_certification=false

## Migration Note

This fix only affects future policy assignments. The 4 Diy nodes on
mainnet farm 2995 currently assigned policy 3 need a migration or
manual set_node_certification call to trigger re-evaluation.

Fixes #429

* fix(ci): use $GITHUB_WORKSPACE for git safe.directory in weights workflow (#1091)

The repository rename (tfchain -> ledger_chain) changed the runner checkout
path to /__w/ledger_chain/ledger_chain, but the safe.directory exception was
hardcoded to /__w/tfchain/tfchain, so git still rejected the repo with
"detected dubious ownership" (exit 128) at the Git config step.

Use $GITHUB_WORKSPACE so the exception always matches the real checkout path,
regardless of repository name.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant