diff --git a/app/app.go b/app/app.go index 84dd7e3..55cd1f5 100644 --- a/app/app.go +++ b/app/app.go @@ -2,6 +2,7 @@ package app import ( "encoding/json" + "errors" "fmt" "io" "maps" @@ -78,6 +79,7 @@ import ( servertypes "github.com/cosmos/cosmos-sdk/server/types" testdata_pulsar "github.com/cosmos/cosmos-sdk/testutil/testdata/testpb" sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/types/module" "github.com/cosmos/cosmos-sdk/types/msgservice" signingtypes "github.com/cosmos/cosmos-sdk/types/tx/signing" @@ -1046,29 +1048,60 @@ func (app *TacChainApp) configureEVMMempool(appOpts servertypes.AppOptions, logg } func (app *TacChainApp) broadcastEVMTransactions(ethTxs []*ethtypes.Transaction) error { + signer := ethtypes.LatestSigner(evmvmtypes.GetEthChainConfig()) + baseDenom := evmvmtypes.GetEVMCoinDenom() + + // One bad transaction must not cost the rest of the batch its broadcast, so + // failures are collected and reported once at the end. + var errs []error + for _, ethTx := range ethTxs { + // The sender has to be recovered and set here: MsgEthereumTx.ValidateBasic + // rejects a message without it, so a broadcast built with FromEthereumTx + // alone is refused by the receiving mempool with "sender address is + // missing". Locally that goes unnoticed, since block proposal reads this + // node's mempool directly, but the transaction never reaches its peers. msg := &evmvmtypes.MsgEthereumTx{} - msg.FromEthereumTx(ethTx) + if err := msg.FromSignedEthereumTx(ethTx, signer); err != nil { + errs = append(errs, fmt.Errorf("failed to recover sender of transaction %s: %w", ethTx.Hash().Hex(), err)) + continue + } - txBuilder := app.txConfig.NewTxBuilder() - if err := txBuilder.SetMsgs(msg); err != nil { - return fmt.Errorf("failed to set msg in tx builder: %w", err) + // Build through the message itself rather than filling a builder by hand: + // an EVM message only passes the ante handler inside a tx carrying the + // ExtensionOptionsEthereumTx option, and the fee and gas limit have to be + // taken off the transaction. Assembling this here by hand is what left + // them out and got the broadcast refused with "MsgEthereumTx needs to be + // contained within a tx with 'ExtensionOptionsEthereumTx' option". + cosmosTx, err := msg.BuildTx(app.txConfig.NewTxBuilder(), baseDenom) + if err != nil { + errs = append(errs, fmt.Errorf("failed to build cosmos tx for %s: %w", ethTx.Hash().Hex(), err)) + continue } - txBytes, err := app.txConfig.TxEncoder()(txBuilder.GetTx()) + txBytes, err := app.txConfig.TxEncoder()(cosmosTx) if err != nil { - return fmt.Errorf("failed to encode transaction: %w", err) + errs = append(errs, fmt.Errorf("failed to encode transaction %s: %w", ethTx.Hash().Hex(), err)) + continue } res, err := app.clientCtx.BroadcastTxSync(txBytes) if err != nil { - return fmt.Errorf("failed to broadcast transaction %s: %w", ethTx.Hash().Hex(), err) + errs = append(errs, fmt.Errorf("failed to broadcast transaction %s: %w", ethTx.Hash().Hex(), err)) + continue + } + // The submitting path has already handed this transaction to Comet, so + // this node's own cache answers the peer broadcast with "tx already in + // mempool". That is the expected outcome here, not a failure. + if res.Code == sdkerrors.ErrTxInMempoolCache.ABCICode() { + continue } if res.Code != 0 { - return fmt.Errorf("transaction %s rejected by mempool: code=%d, log=%s", ethTx.Hash().Hex(), res.Code, res.RawLog) + errs = append(errs, fmt.Errorf("transaction %s rejected by mempool: code=%d, log=%s", ethTx.Hash().Hex(), res.Code, res.RawLog)) } } - return nil + + return errors.Join(errs...) } func (app *TacChainApp) setPostHandler() { diff --git a/app/evm_mempool_broadcast_test.go b/app/evm_mempool_broadcast_test.go index 1d87c5e..dc20e56 100644 --- a/app/evm_mempool_broadcast_test.go +++ b/app/evm_mempool_broadcast_test.go @@ -17,15 +17,44 @@ import ( cmttypes "github.com/cometbft/cometbft/types" "github.com/cosmos/cosmos-sdk/client" simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims" + sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + authante "github.com/cosmos/cosmos-sdk/x/auth/ante" "github.com/cosmos/evm/mempool/txpool/legacypool" evmtypes "github.com/cosmos/evm/x/vm/types" ethcmn "github.com/ethereum/go-ethereum/common" ethtypes "github.com/ethereum/go-ethereum/core/types" + ethcrypto "github.com/ethereum/go-ethereum/crypto" ) +// signedEthTx builds a transaction the broadcast path can recover a sender from. +// An unsigned one is not usable here: MsgEthereumTx.ValidateBasic rejects a +// message without a sender, so the broadcast has to derive it from the signature. +func signedEthTx(t *testing.T) (*ethtypes.Transaction, ethcmn.Address) { + t.Helper() + + key, err := ethcrypto.GenerateKey() + require.NoError(t, err) + + to := ethcmn.Address{} + tx, err := ethtypes.SignNewTx(key, ethtypes.LatestSigner(evmtypes.GetEthChainConfig()), ðtypes.LegacyTx{ + Nonce: 1, + To: &to, + Value: big.NewInt(0), + Gas: 21_000, + GasPrice: big.NewInt(1), + }) + require.NoError(t, err) + + return tx, ethcrypto.PubkeyToAddress(key.PublicKey) +} + type broadcastRecorder struct { rpcmock.Client + // code the node answers the broadcast with; zero means accepted + code uint32 + mu sync.Mutex calls int tx cmttypes.Tx @@ -37,7 +66,14 @@ func (r *broadcastRecorder) BroadcastTxSync(_ context.Context, tx cmttypes.Tx) ( r.calls++ r.tx = append(r.tx[:0], tx...) - return &coretypes.ResultBroadcastTx{}, nil + return &coretypes.ResultBroadcastTx{Code: r.code}, nil +} + +func (r *broadcastRecorder) setCode(code uint32) { + r.mu.Lock() + defer r.mu.Unlock() + r.code = code + r.calls = 0 } func (r *broadcastRecorder) callCount() int { @@ -87,14 +123,7 @@ func TestEVMMempoolBroadcastTxFnUsesUpdatedClientCtx(t *testing.T) { WithClient(rpcClient), ) - to := ethcmn.Address{} - ethTx := ethtypes.NewTx(ðtypes.LegacyTx{ - Nonce: 1, - To: &to, - Value: big.NewInt(0), - Gas: 21_000, - GasPrice: big.NewInt(1), - }) + ethTx, sender := signedEthTx(t) // Before the explicit BroadCastTxFn override, this callback captured the // empty client.Context from app construction and returned "no RPC client is @@ -117,6 +146,27 @@ func TestEVMMempoolBroadcastTxFnUsesUpdatedClientCtx(t *testing.T) { msg, ok := msgs[0].(*evmtypes.MsgEthereumTx) require.True(t, ok) require.Equal(t, ethTx.Hash(), msg.Hash()) + + // Without the sender the receiving mempool refuses the message with + // "sender address is missing", so the transaction never reaches its peers. + require.Equal(t, sender.Bytes(), []byte(msg.From), + "broadcast message must carry the recovered sender") + require.NoError(t, msg.ValidateBasic()) + + // The ante handler routes a transaction to the EVM path by this extension + // option and nothing else, so a broadcast without it is refused before the + // message is even looked at. + extTx, ok := decodedTx.(authante.HasExtensionOptionsTx) + require.True(t, ok) + opts := extTx.GetExtensionOptions() + require.Len(t, opts, 1, "broadcast tx must carry the ethereum extension option") + require.Equal(t, "/cosmos.evm.vm.v1.ExtensionOptionsEthereumTx", opts[0].GetTypeUrl()) + + // Fee and gas live on the ethereum transaction and have to be carried over. + feeTx, ok := decodedTx.(sdk.FeeTx) + require.True(t, ok) + require.Equal(t, ethTx.Gas(), feeTx.GetGas()) + require.False(t, feeTx.GetFee().IsZero(), "broadcast tx must carry a fee") } func TestEVMMempoolBroadcastTxFnDoesNotBlockOnBroadcast(t *testing.T) { @@ -144,14 +194,7 @@ func TestEVMMempoolBroadcastTxFnDoesNotBlockOnBroadcast(t *testing.T) { WithClient(rpcClient), ) - to := ethcmn.Address{} - ethTx := ethtypes.NewTx(ðtypes.LegacyTx{ - Nonce: 1, - To: &to, - Value: big.NewInt(0), - Gas: 21_000, - GasPrice: big.NewInt(1), - }) + ethTx, _ := signedEthTx(t) done := make(chan error, 1) go func() { @@ -178,3 +221,46 @@ func TestEVMMempoolBroadcastTxFnDoesNotBlockOnBroadcast(t *testing.T) { t.Fatal("background broadcast goroutine did not exit") } } + +func TestBroadcastEVMTransactionsMempoolCodes(t *testing.T) { + tacApp := NewTacChainAppWithCustomOptions(t, true, SetupOptions{ + Logger: log.NewTestLogger(t), + DB: dbm.NewMemDB(), + AppOpts: simtestutil.NewAppOptionsWithFlagHome(t.TempDir()), + }) + + ethTx, _ := signedEthTx(t) + + // RegisterTxService may only run once per app, so the same client is reused + // and its answer changed per case. + rpcClient := &broadcastRecorder{} + tacApp.RegisterTxService(client.Context{}. + WithTxConfig(tacApp.txConfig). + WithClient(rpcClient), + ) + + for _, tc := range []struct { + name string + code uint32 + wantErr bool + }{ + // The submitting path has already given this transaction to Comet, so + // the node's own cache answers the peer broadcast as a duplicate. That + // is the normal outcome, not something to report. + {"duplicate in mempool cache", sdkerrors.ErrTxInMempoolCache.ABCICode(), false}, + {"accepted", 0, false}, + {"rejected", sdkerrors.ErrInvalidRequest.ABCICode(), true}, + } { + t.Run(tc.name, func(t *testing.T) { + rpcClient.setCode(tc.code) + + err := tacApp.broadcastEVMTransactions([]*ethtypes.Transaction{ethTx}) + if tc.wantErr { + require.Error(t, err) + } else { + require.NoError(t, err) + } + require.Equal(t, 1, rpcClient.callCount()) + }) + } +} diff --git a/app/upgrades.go b/app/upgrades.go index 57477dd..57fb865 100644 --- a/app/upgrades.go +++ b/app/upgrades.go @@ -27,7 +27,7 @@ var Upgrades = []upgrades.Upgrade{ v104.Upgrade, // ed25519 precompile v160.Upgrade, // upgrade to cosmos/evm v0.6.0 v160spbhotfix.Upgrade, - v161.Upgrade, // vesting schedule shift (+6mo) & vesting account migration + v161.Upgrade, // no-op handler; binary-level fixes only } // RegisterUpgradeHandlers registers the chain upgrade handlers diff --git a/docs/CHANGELOG-v160-v161.md b/docs/CHANGELOG-v160-v161.md index 73547de..f53f187 100644 --- a/docs/CHANGELOG-v160-v161.md +++ b/docs/CHANGELOG-v160-v161.md @@ -1,66 +1,68 @@ # TacChain v1.6.1 — Changelog -> **Status:** DRAFT +> **Status:** Ready for release > **Upgrade name:** `v1.6.1` +> **Release tag:** `v1.6.1` (to be cut from `main`) > **Previous version:** v1.6.0 -> **Chain:** TacChain Mainnet +> **Chains:** SPB testnet — upgraded 2026-07-30 at height 23,205,042 with +> `v1.6.1-beta.1`; TacChain Mainnet — planned --- ## Summary -TacChain v1.6.1 is a maintenance release on top of v1.6.0. It carries **no state -migration**: the upgrade handler exists only so the network can coordinate a -version bump to ship binary-level fixes. +Maintenance release on top of v1.6.0. No state migration, no consensus-affecting +change: the handler exists only to coordinate a version bump for binary fixes. -Everything in this release comes from the `cosmos/evm` and `cosmos-sdk` forks: +--- + +## Rollout + +`v1.6.1` is **not** byte-identical to the `v1.6.1-beta.1` pre-release running on SPB. + +| Build | `cosmos/evm` fork | `cosmos-sdk` fork | Where | +|-------|-------------------|-------------------|-------| +| `v1.6.1-beta.1` | `v0.6.0-tac.10` | `v0.53.6-tac.3` | SPB testnet since 2026-07-30 | +| `v1.6.1` | `v0.6.0-tac.13` | `v0.53.6-tac.3` | mainnet target | + +In `v1.6.1`, not in `v1.6.1-beta.1`: -- Delegating vesting-locked tokens through the EVM staking precompile no longer - burns coins or panics -- Historical EVM queries below the v1.6.0 store-migration height work again -- A state override on `eth_call` no longer drops the static precompiles -- New `tac_simulate` JSON-RPC method +| Change | Commit | Path | +|--------|--------|------| +| `tac_simulate` JSON-RPC method | `9a602662` | query | +| Balance override reaches gas estimation | `5f530858` | query | +| Static precompiles kept under an `eth_call` state override | `96d5164b` | query | +| EVM transaction broadcast to peers | `6759d93`, `548387b`, `b87bd40` | p2p | + +None affects execution or consensus, so the SPB run covers the consensus-relevant +part of the release. --- ## Breaking Changes -None. No consensus-affecting change, no state migration, no parameter change. +None. No state migration, no parameter change, no consensus version bump. --- ## New Features -### RPC - -- **`tac_simulate`** — a simulated, non-committing call that reports in one - round-trip what `eth_call`, `eth_estimateGas` and a log inspection would take - three of: - - ```json - { - "success": true, - "output": "0x...", - "vmError": "", - "logs": [], - "gasEstimated": "0x..." - } - ``` - - Parameters mirror `eth_call`: call args, block number or hash, and state - overrides. - - - **Event logs** emitted during the simulation are returned in `logs`. - - **`gasEstimated`** is a full `eth_estimateGas` binary search run against the - *same* overridden state as the call, so an estimate can be produced for state - that does not exist on chain yet. - - **A revert is a result, not an error.** Where `eth_call` turns a revert into - a JSON-RPC error, `tac_simulate` returns `success: false` together with - `vmError` and the revert data in `output`, so a caller can decode a custom - error instead of just seeing the request fail. - - Served under the **`tac` namespace, which is not enabled by default** — it has - to be listed in `app.toml` under `[json-rpc] api` to be reachable. +### `tac_simulate` (JSON-RPC) + +Non-committing simulated call. Parameters mirror `eth_call` (call args, block +number or hash, state overrides). Returns in one round-trip: + +```json +{ "success": true, "output": "0x...", "vmError": "", "logs": [], "gasEstimated": "0x..." } +``` + +- `logs` — events emitted during simulation. +- `gasEstimated` — full `eth_estimateGas` search against the same overridden state. +- A revert returns `success: false` with `vmError` and revert data in `output`, + instead of a JSON-RPC error. + +Served under the `tac` namespace, **not enabled by default** — must be listed in +`app.toml` under `[json-rpc] api`. --- @@ -68,111 +70,90 @@ None. No consensus-affecting change, no state migration, no parameter change. ### Vesting delegations via the EVM staking precompile -Delegating vesting-**locked** tokens through the staking precompile either -silently burned coins (when `amount <= spendable`) or aborted with an -integer-overflow panic (when `amount > spendable`). - -The EVM balance handler subtracted the full `CoinSpent` amount from an EVM -balance that only ever reflects the *spendable* portion, while a delegation from -a vesting account is drawn from the locked portion and does not reduce spendable -at all. +Delegating vesting-**locked** tokens silently burned coins (`amount <= spendable`) +or panicked on integer overflow (`amount > spendable`): the EVM balance handler +subtracted the full `CoinSpent` from a balance that only reflects the spendable +portion. -`x/bank` now tags `coin_spent` with a `locked_amount` attribute -(`cosmos-sdk` fork), and the handler subtracts only the spendable part -(`amount - locked`). An inconsistent event where `locked > amount` is now an -error rather than a silent clamp. `SubBalance` is additionally guarded against -underflow (upstream `cosmos/evm` #1176). +`x/bank` now tags `coin_spent` with `locked_amount`, and the handler subtracts +`amount - locked`. `locked > amount` is an error, not a silent clamp. `SubBalance` +guarded against underflow (upstream `cosmos/evm` #1176). -> Requires both fork bumps together — the EVM-side handler depends on the -> `locked_amount` emission from the SDK side. +> Needs both fork bumps together — the EVM handler depends on the SDK-side emission. ### Historical EVM queries below the v1.6.0 migration height -`eth_call`, `eth_estimateGas` and traces at heights **below** the v1.6.0 store -migration failed or returned wrong results, because pre-migration state was -being decoded with the current, incompatible layout: +`eth_call`, `eth_estimateGas` and traces below the v1.6.0 store migration failed +or returned wrong results — pre-migration state was decoded with the current layout: -- **`x/vm` params** — proto field numbers shifted in v1.6.0 (old field 10 - `active_static_precompiles` vs. new `history_serve_window`), so decoding - panicked with `wrong wireType = 2 for field HistoryServeWindow`. -- **`x/erc20` precompiles** — the native/dynamic lists moved from a single - concatenated-blob key to per-address keys, so precompiles looked unregistered - at historical heights and calls executed the decoy ERC20 bytecode instead of - the precompile. In practice `gTAC.balanceOf` at an old height returned `0` - instead of the real balance. +- `x/vm` params — field numbers shifted in v1.6.0, decoding panicked with + `wrong wireType = 2 for field HistoryServeWindow`. +- `x/erc20` precompiles — lists moved from a concatenated-blob key to per-address + keys, so precompiles looked unregistered and calls hit the decoy ERC20 bytecode + (`gTAC.balanceOf` at an old height returned `0`). -A lazy, height-gated shim now decodes the legacy layout on the read path below -the migration height. The height is resolved from the applied `x/upgrade` -done-height and cached; the caches are warmed in the `x/vm` `BeginBlock`. +A height-gated shim now decodes the legacy layout on the read path; the height +comes from the applied `x/upgrade` done-height, caches warmed in `x/vm` `BeginBlock`. -> **Query path only.** At current heights the code path is byte-for-byte -> unchanged, so this is not consensus-affecting. +> Query path only — at current heights the code path is byte-for-byte unchanged. ### State overrides dropped the static precompiles -Backport of upstream [cosmos/evm #1096](https://github.com/cosmos/evm/pull/1096). +Backport of upstream [cosmos/evm #1096](https://github.com/cosmos/evm/pull/1096), +fixed in 0.7.x and never backported to 0.6.x, so **v1.6.0 is affected**. -An `eth_call` carrying state overrides replaced the whole precompile set with -go-ethereum's stock one, so the chain's static precompiles were not installed. -A call to a precompile address then landed on an account with no code and came -back with **empty output, intrinsic-only gas and no error** — a silent wrong -answer rather than a failure. +An `eth_call` with state overrides — an empty `{}` was enough — replaced the +precompile set with go-ethereum's stock one. Calls to `0x800` (staking), `0x801` +(distribution), `0x804` (bank) returned empty output, intrinsic-only gas, no error. -An empty `{}` was enough to trigger it, which is what most tooling sends when it -passes the override argument at all. +`tac_simulate` inherited it: gas estimate ~4.6x too low on precompile calls +(`delegate`: 25 390 vs 115 613). Both agree after the backport. -Upstream fixed this in 0.7.x in April and never backported it to 0.6.x, so -**v1.6.0 is affected** and any `eth_call` with overrides against `0x800` -(staking), `0x801` (distribution), `0x804` (bank) and friends has been returning -`0x` there. +> Query path only — transactions never carry overrides. -> Query path only — a transaction never carries overrides, so nothing about -> execution or consensus changes. +### Gas estimation with a balance override -`tac_simulate` inherited the same problem, which made its gas estimate ~4.6x too -low on precompile calls (measured on a `delegate`: 25 390 against the 115 613 -`eth_estimateGas` reports). Both agree after the backport. +The gas-search upper bound is capped by what the sender can pay, and that balance +was read off the bank keeper, never the state override — so pricing a call for an +unfunded account returned `gasEstimated: 0` while the call itself succeeded. -### Gas estimation with a balance override +The balance now comes from the override when it carries one. No override, no change. -The upper bound of the gas estimation binary search is capped by what the sender -can pay for gas, and that balance was read straight off the bank keeper — the -state override never reached it. Pricing a call for an account that is not funded -yet therefore returned no estimate at all: the call itself succeeded on the -overridden state while `gasEstimated` came back `0`. +> Only observable when the caller passes a gas price. -The balance now comes from the override when it carries one for the sender, the -way go-ethereum reads it off the overridden state. Without an override nothing -changes, so `eth_estimateGas` keeps its behaviour. +### EVM transaction broadcast to peers -> Only observable when the caller passes a gas price; otherwise the fee cap -> defaults to `0` and the recap is skipped entirely. +The broadcast message was assembled by hand and refused by the receiving mempool: +no sender (`code=18`), then no `ExtensionOptionsEthereumTx` option, fee or gas +limit (`code=29`). It is now built through `MsgEthereumTx.BuildTx`, as +`SendRawTransaction` does. A duplicate from the node's own cache no longer counts +as a failure, and one bad transaction no longer aborts its batch. + +Transactions still landed (a proposing node reads its own mempool) but did not +reach other validators. + +> p2p propagation only, not execution. --- -## Upgrade Handler Details +## Upgrade Handler -`v1.6.1` is a **no-op state upgrade**. The handler runs the standard module -migrations and returns: +No-op: runs module migrations and returns. -- No `StoreUpgrades` — no store added, renamed or deleted. -- No KV migration. -- No parameter change. +- No `StoreUpgrades`, no KV migration, no parameter change. - No `ConsensusVersion` bump in any module. - -A vesting schedule shift and an account migration were considered for this -release and **dropped** — they were not approved — so no vesting logic ships in -the handler. +- Vesting schedule shift and account migration were considered and dropped — not + approved, no vesting logic ships. --- ## Documentation & Tooling -- Cosmovisor setup guide now covers migrating an already running node. -- Deprecated Turin testnet (`tacchain_2390-1`) removed: `NETWORKS.md` section and - the `networks/tacchain_2390-1/` directory (genesis, compose file, env). -- The localnet script now lists `tac` in `json-rpc.api`, so `tac_simulate` is - reachable out of the box on a locally initialised chain. +- Cosmovisor setup guide covers migrating an already running node. +- Deprecated Turin testnet (`tacchain_2390-1`) removed from `NETWORKS.md` and + `networks/tacchain_2390-1/`. +- Localnet script lists `tac` in `json-rpc.api`, so `tac_simulate` works out of the box. +- `gen_localnode.sh` — stray `exit` cut the script short before the docker build. --- @@ -187,8 +168,6 @@ the handler. | `cosmos/ibc-go/v10` | `v10.3.1` | unchanged | | Go | 1.23.8 | unchanged | -### Fork changes in detail - **`cosmos/evm` `v0.6.0-tac.8` → `v0.6.0-tac.13`** | Commit | Change | diff --git a/gen_localnode.sh b/gen_localnode.sh index 30e46f6..5dc482e 100755 --- a/gen_localnode.sh +++ b/gen_localnode.sh @@ -30,7 +30,6 @@ COPYFILE_DISABLE=1 tar --no-xattrs --format=ustar \ -czf .tacchaind.tar \ .tacchaind -exit docker buildx build \ --platform linux/amd64 \ --load \