Skip to content

Commit 3d1145f

Browse files
sameh-faroukclaude
andauthored
fix(pallet-tfgrid): enforce node_certification in farming policy limits (#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>
1 parent 0274227 commit 3d1145f

2 files changed

Lines changed: 159 additions & 11 deletions

File tree

substrate-node/pallets/pallet-tfgrid/src/pricing.rs

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -196,18 +196,27 @@ impl<T: Config> Pallet<T> {
196196

197197
// If there is a farming policy defined on the
198198
// farm policy limits, use that one
199-
match farm.farming_policy_limits {
199+
match farm.farming_policy_limits.clone() {
200200
Some(mut limits) => {
201201
ensure!(
202202
FarmingPoliciesMap::<T>::contains_key(limits.farming_policy_id),
203203
Error::<T>::FarmingPolicyNotExists
204204
);
205+
206+
// If the policy limit requires certified nodes, non-certified
207+
// nodes get a default policy matching their certification.
208+
if limits.node_certification
209+
&& node.certification != NodeCertification::Certified
210+
{
211+
return Self::get_default_farming_policy_for(node, &farm);
212+
}
213+
205214
match limits.end {
206215
Some(end_timestamp) => {
207216
let now =
208217
<pallet_timestamp::Pallet<T>>::get().saturated_into::<u64>() / 1000;
209218
if now > end_timestamp {
210-
return Self::get_default_farming_policy();
219+
return Self::get_default_farming_policy_for(node, &farm);
211220
}
212221
}
213222
None => (),
@@ -217,7 +226,7 @@ impl<T: Config> Pallet<T> {
217226
Some(cu_limit) => {
218227
let cu = node.resources.get_cu();
219228
if cu > cu_limit {
220-
return Self::get_default_farming_policy();
229+
return Self::get_default_farming_policy_for(node, &farm);
221230
}
222231
limits.cu = Some(cu_limit - cu);
223232
}
@@ -228,7 +237,7 @@ impl<T: Config> Pallet<T> {
228237
Some(su_limit) => {
229238
let su = node.resources.get_su();
230239
if su > su_limit {
231-
return Self::get_default_farming_policy();
240+
return Self::get_default_farming_policy_for(node, &farm);
232241
}
233242
limits.su = Some(su_limit - su);
234243
}
@@ -238,7 +247,7 @@ impl<T: Config> Pallet<T> {
238247
match limits.node_count {
239248
Some(node_count) => {
240249
if node_count == 0 {
241-
return Self::get_default_farming_policy();
250+
return Self::get_default_farming_policy_for(node, &farm);
242251
}
243252
limits.node_count = Some(node_count - 1);
244253
}
@@ -288,21 +297,25 @@ impl<T: Config> Pallet<T> {
288297
}
289298
}
290299

291-
// Set the default farming policy as the last best certified
292-
// farming policy amoung all the default farming policies
293-
fn get_default_farming_policy(
300+
// Select the best matching default farming policy for the given node
301+
// and farm certification levels.
302+
fn get_default_farming_policy_for(
303+
node: &TfgridNode<T>,
304+
farm: &FarmInfoOf<T>,
294305
) -> Result<types::FarmingPolicy<BlockNumberFor<T>>, DispatchErrorWithPostInfo> {
295306
let mut policies: Vec<types::FarmingPolicy<BlockNumberFor<T>>> =
296307
FarmingPoliciesMap::<T>::iter().map(|p| p.1).collect();
297308

298309
policies.sort();
299-
// by reversing sorted policies we place default policies first
300-
// and then rank them from more certified to less certified
301310
policies.reverse();
302311

303312
let possible_policy = policies
304313
.into_iter()
305-
.filter(|policy| policy.default)
314+
.filter(|policy| {
315+
policy.default
316+
&& policy.node_certification <= node.certification
317+
&& policy.farm_certification <= farm.certification
318+
})
306319
.take(1)
307320
.next();
308321

substrate-node/pallets/pallet-tfgrid/src/tests.rs

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1783,6 +1783,141 @@ fn attach_farming_policy_with_certified_node_certification_works() {
17831783
});
17841784
}
17851785

1786+
#[test]
1787+
fn diy_node_gets_default_policy_when_limit_requires_certification() {
1788+
ExternalityBuilder::build().execute_with(|| {
1789+
create_twin();
1790+
create_farm();
1791+
create_node();
1792+
1793+
let farm_id = 1;
1794+
let node_id = 1;
1795+
1796+
// Confirm node starts as Diy
1797+
let node = TfgridModule::nodes(node_id).unwrap();
1798+
assert_eq!(node.certification, NodeCertification::Diy);
1799+
1800+
// Attach farming policy 3 (certified) with node_certification = true
1801+
let fp = TfgridModule::farming_policies_map(3);
1802+
let limit = FarmingPolicyLimit {
1803+
farming_policy_id: fp.id,
1804+
cu: Some(100),
1805+
su: Some(100),
1806+
end: None,
1807+
node_certification: true,
1808+
node_count: Some(100),
1809+
};
1810+
1811+
assert_ok!(TfgridModule::attach_policy_to_farm(
1812+
RawOrigin::Root.into(),
1813+
farm_id,
1814+
Some(limit)
1815+
));
1816+
1817+
// Diy node should NOT get the certified policy (3) — should fall back
1818+
// to the best matching default for a Diy node on a Gold farm, which is
1819+
// policy 1 (Diy + Gold)
1820+
let node = TfgridModule::nodes(node_id).unwrap();
1821+
assert_ne!(
1822+
node.farming_policy_id, fp.id,
1823+
"Diy node should not receive certified-only farming policy"
1824+
);
1825+
assert_eq!(
1826+
node.farming_policy_id, 1,
1827+
"Diy node on Gold farm should get policy 1 (Diy + Gold default)"
1828+
);
1829+
});
1830+
}
1831+
1832+
#[test]
1833+
fn certified_node_gets_limited_policy_when_limit_requires_certification() {
1834+
ExternalityBuilder::build().execute_with(|| {
1835+
create_twin();
1836+
create_farm();
1837+
create_node();
1838+
1839+
let farm_id = 1;
1840+
let node_id = 1;
1841+
1842+
// Certify the node first
1843+
assert_ok!(TfgridModule::add_node_certifier(
1844+
RawOrigin::Root.into(),
1845+
alice()
1846+
));
1847+
assert_ok!(TfgridModule::set_node_certification(
1848+
RuntimeOrigin::signed(alice()),
1849+
node_id,
1850+
NodeCertification::Certified
1851+
));
1852+
let node = TfgridModule::nodes(node_id).unwrap();
1853+
assert_eq!(node.certification, NodeCertification::Certified);
1854+
1855+
// Attach certified-only policy
1856+
let fp = TfgridModule::farming_policies_map(3);
1857+
let limit = FarmingPolicyLimit {
1858+
farming_policy_id: fp.id,
1859+
cu: Some(100),
1860+
su: Some(100),
1861+
end: None,
1862+
node_certification: true,
1863+
node_count: Some(100),
1864+
};
1865+
1866+
assert_ok!(TfgridModule::attach_policy_to_farm(
1867+
RawOrigin::Root.into(),
1868+
farm_id,
1869+
Some(limit)
1870+
));
1871+
1872+
// Certified node SHOULD get the limited policy
1873+
let node = TfgridModule::nodes(node_id).unwrap();
1874+
assert_eq!(
1875+
node.farming_policy_id, fp.id,
1876+
"Certified node should receive the certified-only farming policy"
1877+
);
1878+
});
1879+
}
1880+
1881+
#[test]
1882+
fn diy_node_gets_limited_policy_when_no_certification_required() {
1883+
ExternalityBuilder::build().execute_with(|| {
1884+
create_twin();
1885+
create_farm();
1886+
create_node();
1887+
1888+
let farm_id = 1;
1889+
let node_id = 1;
1890+
1891+
// Confirm node is Diy
1892+
let node = TfgridModule::nodes(node_id).unwrap();
1893+
assert_eq!(node.certification, NodeCertification::Diy);
1894+
1895+
// Attach policy with node_certification = false (no restriction)
1896+
let fp = TfgridModule::farming_policies_map(3);
1897+
let limit = FarmingPolicyLimit {
1898+
farming_policy_id: fp.id,
1899+
cu: Some(100),
1900+
su: Some(100),
1901+
end: None,
1902+
node_certification: false,
1903+
node_count: Some(100),
1904+
};
1905+
1906+
assert_ok!(TfgridModule::attach_policy_to_farm(
1907+
RawOrigin::Root.into(),
1908+
farm_id,
1909+
Some(limit)
1910+
));
1911+
1912+
// Diy node should get the limited policy since certification is not required
1913+
let node = TfgridModule::nodes(node_id).unwrap();
1914+
assert_eq!(
1915+
node.farming_policy_id, fp.id,
1916+
"Diy node should receive limited policy when node_certification is false"
1917+
);
1918+
});
1919+
}
1920+
17861921
#[test]
17871922
fn attach_another_custom_farming_policy_to_farm_works() {
17881923
ExternalityBuilder::build().execute_with(|| {

0 commit comments

Comments
 (0)