MNT Part 1: feat(types): add QuarkChain MNT account and token types - #24
MNT Part 1: feat(types): add QuarkChain MNT account and token types#24ping-ke wants to merge 15 commits into
Conversation
|
General note: the three inline issues above were all found in a first-pass Codex 5.5 Extra High review with no additional prompt engineering beyond asking it to review this PR. Before requesting review next time, please run a self-review pass with Codex 5.5 Extra High (or an equivalent high-reasoning review mode) and address the obvious state/snapshot/generation issues it finds first. |
3860cb7 to
53e97c4
Compare
53e97c4 to
56fa444
Compare
56fa444 to
61adf84
Compare
- Add TokenBalances type with sorted list encoding compatible with pyquarkchain - Add StateAccount.MntBalances field and QKC 6-element RLP codec (replaces generated gen_account_rlp.go with hand-written EncodeRLP/DecodeRLP) - Add uint32 RLP encoding helpers for token IDs - Update genesis hashes to reflect QKC 6-element account encoding - Add comprehensive tests: roundtrip, pyquarkchain encode/decode compatibility
Comment 1: SlimAccount only held {Nonce,Balance,Root,CodeHash}, so the
slim-RLP path used by SlimAccountRLP (stateupdate.go account updates/
origins), FullAccount (pathdb rollback in triedb/pathdb/execute.go) and
flatReader.Account (snapshot flat read) silently dropped MntBalances and
FullShardKey. A QKC account served from any of those paths came back with
MntBalances=nil / FullShardKey=0 and re-committed a corrupted account,
forking the trie root.
Extend SlimAccount with:
- MntBal []byte (rlp optional) = TokenBalances.SerializeToBytes()
- FullShardKey uint32 (rlp optional)
MntBal uses the []byte serialization (TokenBalances holds an unexported
map, not RLP-struct-encodable) and preserves the nil-vs-empty distinction
so the 0x80 / 0x8200c0 trie encoding stays byte-stable across the slim
round-trip. Unlike the trie qkcAccountRLP.TokenBal, the QKC default
balance is NOT merged into MntBal — slim keeps it in the Balance field.
Both fields are rlp optional so pre-MNT snapshots still decode. Because
FullAccount now reconstructs both fields, the pathdb rollback path is
covered without further changes.
Extend TestSlimRLPRoundTripEquivalence with fullShardKey / MNT-only /
MNT+QKC+shard cases (direct-QKC-encode == via-slim-encode).
Comment 3: remove the rlpgen go:generate directive. StateAccount now uses
the hand-written QKC codec (EncodeRLP/DecodeRLP in state_account_qkc.go);
regenerating gen_account_rlp.go would reintroduce a conflicting standard
4-field codec that drops MntBalances / FullShardKey. Replaced the
directive with a NOTE explaining why it must stay removed.
Comment 2 (empty() must consider MntBalances) is resolved downstream on
feature/mnt-state (stateObject.empty() checks IsBlankMnt(), covered by
TestEmptyAccountWithMntNotPruned); core/state is not part of this branch.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…reum genesis hashes
Fixes found while reviewing the MNT account encoding: - EncodeRLP no longer dereferences a nil *TokenBalances. When Balance is nil (or zero) and MntBalances is non-nil but empty, mergeQKCTokenBalances returns nil, and the old three-branch switch fell through to calling SerializeToBytes on that nil receiver. Collapsing the switch into a single nil-guarded path removes the whole class of gap. - DecodeRLP rejects a non-empty optional field instead of silently dropping it. pyquarkchain's _Account always writes b"" there, so a non-empty value could only come from a foreign encoder, and discarding it would change the bytes on re-encode. - Dropped the dead rlp:"optional" tag on StateAccount.MntBalances. The type has a hand-written codec so the tag never applied, and as written it was invalid (an optional field followed by the non-optional FullShardKey), which would break any future codec built for this struct. - Removed qkc/common/uint32_rlp.go: qkc/common/special_rlp.go now provides Uint32 after it moved down from qkc/types. Deliberately unchanged: the zero-valued-token-balance encoding is non-idempotent (first encode 0x00c0, re-encode empty) because pyquarkchain's TokenBalances.serialize tests len(_balances) before filtering zero balances. Canonicalizing it would fork the account trie root. Pinned by TestStateAccountEmptyBalancesPythonGolden. SlimAccountRLP keeps panicking on a serialization error, matching the surrounding geth convention. Adds TestStateAccountEncodeBalanceMntCombinations, covering Balance (nil/zero/non-zero) against MntBalances (nil/empty/zero-valued/non-zero) and pinning the wire TokenBal for each. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ceef3d7 to
7ef2bab
Compare
Account RLP Compatibility with pyquarkchainThe following tables compare balance-related bytes only. The values were obtained by executing the current Go account workflow and cross-checking the resulting token-balance payloads with pyquarkchain's
The examples use:
New AccountA new account can be initialized with different QKC and MNT balances.
Decode -> Update Nonce -> EncodeUpdating the nonce does not change
Decode -> Update Balance -> EncodeWhen the QKC balance changes and
Conclusion
The final |
|
Checked every form in the tables against There is one input form the tables don't cover, and it's the one that breaks: TokenBal =
Filling that row in:
For the state-transition path that Yes is correct and should stay: pyquarkchain rebuilds The problem is the
Measured on So Suggested split: keep the codec lossless ( Two The pair is |
|
@ping-ke Small correction: the MNT pair hex in the table looks off by one byte. For
The QKC-only value looks correct. |
|
pyquarkchain has two distinct QKC=0 states: an absent token table ( After splitting pyquarkchain's unified token table into That feels fragile: if any caller leaves |
Resolved across the stacked branches.
Consensus |
|
One remaining snapshot-scope concern: if snap sync and pathdb are intentionally out of scope for this PR, that sounds fine, but snapshot database reads still seem in scope.
If this fix is intentionally deferred to the next PR, could you please make that explicit in the PR description? That would make the supported snapshot scope of this PR clear. |
|
Even if snap sync and pathdb are out of scope, the snapshot database still cannot round-trip a 00c0 leaf: SlimAccountRLP stores the marker, but FullAccountRLP drops it during proof verification and trie regeneration. Please preserve the marker for snapshot leaf reconstruction while keeping normal snapshot reads normalized, or explicitly declare these snapshot paths unsupported as well. |
The flat-reader integration is intentionally deferred to the next stacked PR ( This PR only introduces the QKC account codec and the extended slim-account representation. It does not by itself claim support for runtime snapshot database reads, pathdb, or snap sync. I've added a "PRs and Scope" section to the PR description to clarify the scope of this PR and the responsibilities of the following stacked PRs. |
| // balanceUpdated keeps a changed zero QKC balance encoded as 00c0. It remains | ||
| // set after a revert because pyquarkchain restores the previous value by | ||
| // writing it back, preserving the zero-valued token entry. | ||
| balanceUpdated bool |
There was a problem hiding this comment.
As we discussed before, I still think this approach is more complicated and more error-prone than simply using a balance dictionary that includes both MNT and native balances.
Summary
core/types, including a QKC 6-element RLP-encodedStateAccountand aTokenBalancestypegen_account_rlp.goand replace it with hand-writtenEncodeRLP/DecodeRLPto support the QKC custom 6-field RLP formatBackground
The standard Ethereum
StateAccountuses 4-element RLP (Nonce, Balance, Root, CodeHash). The QuarkChain protocol extends this with two additional fields:FullShardKey uint32: the shard key of the accountMntBalances TokenBalances: a map of non-default native token balances held by the accountThis change produces a different state root from standard go-ethereum, requiring the genesis hash constants to be updated accordingly.
Changed Files
core/types/gen_account_rlp.gocore/types/state_account.goFullShardKeyandMntBalancesfields and extendSlimAccountcore/types/state_account_qkc.gocore/types/state_account_qkc_test.goPRs and Scope
feature/mnt-core-types)TokenBalances, 6-element RLP codec, and extendedSlimAccountincore/types. Snap sync wire-format support remains out of scope.feature/mnt-statecore/state, including state objects, journaling, revert/history handling,StateDB, and snapshot reader support.flatReader.AccountpreservesFullShardKeyand decodesMntBalintoMntBalances.feature/mnt-state-test-fixturesAfter review, these three stacked changes will be combined into a single PR and merged.
Key Design Decisions
DefaultTokenID = 35760: Equal to
TokenIDEncode("QKC"), consistent with pyquarkchain. The QKC token balance is stored in the standardBalancefield, not in theMntBalancesmap, to preserve EIP-20 compatibility.Test Plan
go test ./core/types— all pass, including pyquarkchain compatibility vector testsgo build ./...— build successfulAdditional state integration and fixture/golden coverage is provided by the follow-up branches listed above.