平行链可以开启收取手续费

[linj-disanbo]

平行链可以开启收取手续费 #1330

Upgrade Notes / 升级风险说明

1. MinTxFeeRate 需要同步配置：启用该 fork 需要平行链 Mempool.MinTxFeeRate > 0 才真正生效，否则是 no-op。
2. 客户端/钱包兼容性：升级到 fork 高度后，旧客户端发送的 tx.Fee=0 交易会开始失败，SDK 和钱包需要同步更新。
3. 余额检查：平行链 processFee 走 coins 扣 tx.From() 余额，rollup 场景下需要确认平行链 coins 状态里发送方有足够余额，否则可能出现大量交易因扣费失败被回退。

[vipwzw]

整体方向和默认值（-1 → MaxHeight，默认不启用）处理是合理的，向后兼容性 OK。几点建议：

1. 同意 @bysomeone 的修改建议（必须改）

当前写法把 IsFork 单独求值，主链每笔交易都会多做一次 map 查找。建议直接内联到 || 后利用短路：

// 主链需要收手续费，平行链开启 ForkParaFee 后也收取手续费
if (!e.cfg.IsPara() || e.cfg.IsFork(e.height, "ForkParaFee")) && e.cfg.GetMinTxFeeRate() > 0 && !ex.IsFree() {
    feelog, err = e.processFee(tx)
    ...
}

2. types/fork.go 命名风格不一致（小）

紧邻几行用的是小写 setFork，新增的这行用了大写 SetFork。功能上没区别，但建议保持一致：

f.setFork("ForkCheckEthTxSort", 0)
f.setFork("ForkProxyExec", 0)
f.setFork("ForkMaxTxFeeV1", 0)
f.setFork("ForkParaFee", -1)   // 改为小写

3. types/testdata/chain33.toml 末尾多了一空行（小）

+ForkParaFee=-1
+

第二行的空行没必要，去掉更干净。

4. 缺少单元测试（建议补）

这是会改变共识行为的 fork，但 PR 没附带任何针对平行链 fee 路径的测试。建议至少补一个用例覆盖：

• 平行链 + 高度 < ForkParaFee → 不调用 processFee（原行为）
• 平行链 + 高度 >= ForkParaFee + MinTxFeeRate>0 + !IsFree → 调用 processFee
• 平行链 + 高度 >= ForkParaFee + MinTxFeeRate==0 → 仍不收费

5. 升级风险建议在 PR 描述或 CHANGELOG 里说明

• 启用该 fork 同时需要平行链 Mempool.MinTxFeeRate > 0 才真正生效，否则是 no-op。
• 升级到 fork 高度后，旧客户端/钱包发出的 tx.Fee=0 交易会开始失败，SDK 和钱包需要同步更新。
• 平行链上 processFee 走 coins 扣 tx.From() 余额，rollup 场景下需要确认平行链 coins 状态里发送方有足够余额，否则可能出现大量交易因扣费失败被回退，建议在 #1330 里再验证一遍。

[linj-disanbo]

整体方向和默认值（-1 → MaxHeight，默认不启用）处理是合理的，向后兼容性 OK。几点建议：

1. 同意 @bysomeone 的修改建议（必须改）

当前写法把 IsFork 单独求值，主链每笔交易都会多做一次 map 查找。建议直接内联到 || 后利用短路：

// 主链需要收手续费，平行链开启 ForkParaFee 后也收取手续费
if (!e.cfg.IsPara() || e.cfg.IsFork(e.height, "ForkParaFee")) && e.cfg.GetMinTxFeeRate() > 0 && !ex.IsFree() {
    feelog, err = e.processFee(tx)
    ...
}

2. types/fork.go 命名风格不一致（小）

紧邻几行用的是小写 setFork，新增的这行用了大写 SetFork。功能上没区别，但建议保持一致：

f.setFork("ForkCheckEthTxSort", 0)
f.setFork("ForkProxyExec", 0)
f.setFork("ForkMaxTxFeeV1", 0)
f.setFork("ForkParaFee", -1)   // 改为小写

3. types/testdata/chain33.toml 末尾多了一空行（小）

+ForkParaFee=-1
+

第二行的空行没必要，去掉更干净。

4. 缺少单元测试（建议补）

这是会改变共识行为的 fork，但 PR 没附带任何针对平行链 fee 路径的测试。建议至少补一个用例覆盖：

• 平行链 + 高度 < ForkParaFee → 不调用 processFee（原行为）
• 平行链 + 高度 >= ForkParaFee + MinTxFeeRate>0 + !IsFree → 调用 processFee
• 平行链 + 高度 >= ForkParaFee + MinTxFeeRate==0 → 仍不收费

5. 升级风险建议在 PR 描述或 CHANGELOG 里说明

• 启用该 fork 同时需要平行链 Mempool.MinTxFeeRate > 0 才真正生效，否则是 no-op。
• 升级到 fork 高度后，旧客户端/钱包发出的 tx.Fee=0 交易会开始失败，SDK 和钱包需要同步更新。
• 平行链上 processFee 走 coins 扣 tx.From() 余额，rollup 场景下需要确认平行链 coins 状态里发送方有足够余额，否则可能出现大量交易因扣费失败被回退，建议在 平行链手续费问题 #1330 里再验证一遍。

整体方向和默认值（-1 → MaxHeight，默认不启用）处理是合理的，向后兼容性 OK。几点建议：

1. 同意 @bysomeone 的修改建议（必须改）

当前写法把 IsFork 单独求值，主链每笔交易都会多做一次 map 查找。建议直接内联到 || 后利用短路：

// 主链需要收手续费，平行链开启 ForkParaFee 后也收取手续费
if (!e.cfg.IsPara() || e.cfg.IsFork(e.height, "ForkParaFee")) && e.cfg.GetMinTxFeeRate() > 0 && !ex.IsFree() {
    feelog, err = e.processFee(tx)
    ...
}

2. types/fork.go 命名风格不一致（小）

紧邻几行用的是小写 setFork，新增的这行用了大写 SetFork。功能上没区别，但建议保持一致：

f.setFork("ForkCheckEthTxSort", 0)
f.setFork("ForkProxyExec", 0)
f.setFork("ForkMaxTxFeeV1", 0)
f.setFork("ForkParaFee", -1)   // 改为小写

3. types/testdata/chain33.toml 末尾多了一空行（小）

+ForkParaFee=-1
+

第二行的空行没必要，去掉更干净。

4. 缺少单元测试（建议补）

这是会改变共识行为的 fork，但 PR 没附带任何针对平行链 fee 路径的测试。建议至少补一个用例覆盖：

• 平行链 + 高度 < ForkParaFee → 不调用 processFee（原行为）
• 平行链 + 高度 >= ForkParaFee + MinTxFeeRate>0 + !IsFree → 调用 processFee
• 平行链 + 高度 >= ForkParaFee + MinTxFeeRate==0 → 仍不收费

5. 升级风险建议在 PR 描述或 CHANGELOG 里说明

• 启用该 fork 同时需要平行链 Mempool.MinTxFeeRate > 0 才真正生效，否则是 no-op。
• 升级到 fork 高度后，旧客户端/钱包发出的 tx.Fee=0 交易会开始失败，SDK 和钱包需要同步更新。
• 平行链上 processFee 走 coins 扣 tx.From() 余额，rollup 场景下需要确认平行链 coins 状态里发送方有足够余额，否则可能出现大量交易因扣费失败被回退，建议在 平行链手续费问题 #1330 里再验证一遍。

1-4 已经修复, 5另外做了说明

[linj-disanbo]

rollup 场景 如何开启平行链手续费

部署配置平行链: 需要需要开启 ForkParaFee 并 设置 Mempool.MinTxFeeRate > 0 才能开启平行链收取手续费
构造交易: 需要设置 tx.Fee
发送交易的地址: 需要只有平行链的主代币

[bysomeone]

代码层面所有 review 意见（含@bysomeone 和 @vipwzw 的建议）已全部处理：

• IsFork 已内联避免主链多余查找
• setFork 命名与周围保持一致
• testdata 多余空行已清理
• 新增 TestExecFee 覆盖 4 个场景
• PR 描述已补充升级风险说明

LGTM.