Conversation
qzhodl
requested review from
blockchaindevsh,
iteyelmp,
ping-ke,
qizhou and
syntrust
| ); | ||
|
|
||
| function send(address to, uint32 destShard) external payable { | ||
| require(destShard != currentShard(), "use normal transfer on same shard"); |
There was a problem hiding this comment.
What if destShard doesn't exist? Also currentShard() is undefined.
There was a problem hiding this comment.
You're right that currentShard() shouldn't be there, and the EL should be shard-agnostic and not know which shard it's running on. The validity check (whether destShard is a valid shard ID, whether it equals the source shard, etc.) should live at the CL/master layer, not in the EL contract — same pattern as Ethereum staking, where the deposit contract doesn't validate validators; the consensus layer does.
That said, after discussing with Qi, I realized the pure staking-pattern approach has a deeper issue for QKC: if all xshard sends flow through the root chain (the way Ethereum deposits/withdrawals flow through the beacon chain), the root chain becomes a bandwidth hotspot. Ethereum can afford this because staking activity is low-volume (a few hundred deposits per day system-wide), but QKC's xshard could easily be tens of thousands of tx per second — that doesn't fit through a single root chain.
The current QKC design actually handles this correctly: xshard lists flow directly slave-to-slave (peer-to-peer between shards), and the root chain only commits a hash via the mheader. Root-chain bandwidth stays O(mheaders), independent of total xshard volume.
I'm writing a separate comparison doc, going through this trade-off in detail. Will follow up with concrete changes to §4.6 once that's done — likely the master should only coordinate metadata (which mheader, which root-block position, what's the xshard commit hash), not carry payload bytes.
|
|
||
| QKC-specific additions: | ||
|
|
||
| - `engine_getPoSWInfoV1(coinbase, blockNumber) -> { stake, recentMineCount, poswWindowSize }` — lets CL fetch the data needed to compute a PoSW difficulty divider. |
There was a problem hiding this comment.
should engine_getPoSWInfoV1 take a parent block hash instead of a block number?
There was a problem hiding this comment.
Good point — yes, it should be a parent block hash, not a block number.
|
|
||
| **Deterministic xshard ordering must be carefully engineered.** When master routes xshard deposits from a confirmed root block to destination shards, it must ensure deterministic ordering (e.g., by root block height, then mheader index, then `XshardSend.nonce`) so that independently-running shard CLs converge on identical deposit sequences. The cursor in block meta implicitly enforced this in the current design; the new design must enforce it explicitly in master. | ||
|
|
||
| **Root anchor is outside Engine API's model.** `hash_prev_root_block` has no geth counterpart. Encoding it into `extraData` works, but it means EL does not validate it — CL must. This is one more thing for CL to get right. |
There was a problem hiding this comment.
Maybe we can further add a simple assertion:
if len(header.Extra) != 32 {
return errors.New("extraData must be exactly 32 bytes (root anchor hash)")
}There was a problem hiding this comment.
Agree the check should exist, just put it in the CL header validator instead of geth.
| } | ||
|
|
||
| XshardDeposit { | ||
| from: Address (20 bytes) |
There was a problem hiding this comment.
Looks like the from field is never used on the destination side? The EL hook only does AddBalance(deposit.To, deposit.Value). If it's for event indexing or explorers, say so.
It helps to clarify: which fields are consensus-critical (needed for deterministic verification) vs. metadata (nice to have for tooling)? Making that distinction explicit would clarify why the
struct is shaped this way and whether the metadata fields belong in the ExecutionPayload or could live out-of-band.
2 participants
No description provided.