From f95a18b8b8a65baddab07952b8cd413adfa2f4a6 Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Tue, 27 May 2025 11:24:11 -0400 Subject: [PATCH 1/3] Unexport transaction visitor --- plugin/evm/atomic/tx_semantic_verifier.go | 23 ++++++++++++++------ plugin/evm/export_tx_test.go | 26 +++-------------------- plugin/evm/tx_test.go | 7 +----- plugin/evm/vm.go | 21 +++++------------- 4 files changed, 25 insertions(+), 52 deletions(-) diff --git a/plugin/evm/atomic/tx_semantic_verifier.go b/plugin/evm/atomic/tx_semantic_verifier.go index 2acd4ad93f..935eb42b89 100644 --- a/plugin/evm/atomic/tx_semantic_verifier.go +++ b/plugin/evm/atomic/tx_semantic_verifier.go @@ -21,7 +21,7 @@ import ( "github.com/ava-labs/coreth/plugin/evm/upgrade/ap0" ) -var _ Visitor = (*SemanticVerifier)(nil) +var _ Visitor = (*semanticVerifier)(nil) var ( ErrAssetIDMismatch = errors.New("asset IDs in the input don't match the utxo") @@ -44,16 +44,26 @@ type VerifierBackend struct { SecpCache *secp256k1.RecoverCache } -// SemanticVerifier is a visitor that checks the semantic validity of atomic transactions. -type SemanticVerifier struct { +// SemanticVerify checks the semantic validity of atomic transactions. +func (b *VerifierBackend) SemanticVerify(tx *Tx, parent AtomicBlockContext, baseFee *big.Int) error { + return tx.UnsignedAtomicTx.Visit(&semanticVerifier{ + Backend: b, + Tx: tx, + Parent: parent, + BaseFee: baseFee, + }) +} + +// semanticVerifier is a visitor that checks the semantic validity of atomic +// transactions. +type semanticVerifier struct { Backend *VerifierBackend Tx *Tx Parent AtomicBlockContext BaseFee *big.Int } -// ImportTx verifies this transaction is valid. -func (s *SemanticVerifier) ImportTx(utx *UnsignedImportTx) error { +func (s *semanticVerifier) ImportTx(utx *UnsignedImportTx) error { backend := s.Backend ctx := backend.Ctx rules := backend.Rules @@ -176,8 +186,7 @@ func conflicts(backend *VerifierBackend, inputs set.Set[ids.ID], ancestor Atomic return nil } -// ExportTx verifies this transaction is valid. -func (s *SemanticVerifier) ExportTx(utx *UnsignedExportTx) error { +func (s *semanticVerifier) ExportTx(utx *UnsignedExportTx) error { backend := s.Backend ctx := backend.Ctx rules := backend.Rules diff --git a/plugin/evm/export_tx_test.go b/plugin/evm/export_tx_test.go index d0a3aed252..59bde7cb0b 100644 --- a/plugin/evm/export_tx_test.go +++ b/plugin/evm/export_tx_test.go @@ -932,15 +932,7 @@ func TestExportTxSemanticVerify(t *testing.T) { } t.Run(test.name, func(t *testing.T) { - tx := test.tx - exportTx := tx.UnsignedAtomicTx - - err := exportTx.Visit(&atomic.SemanticVerifier{ - Backend: backend, - Tx: tx, - Parent: parent, - BaseFee: test.baseFee, - }) + err := backend.SemanticVerify(test.tx, parent, test.baseFee) if test.shouldErr && err == nil { t.Fatalf("should have errored but returned valid") } @@ -1791,13 +1783,7 @@ func TestNewExportTx(t *testing.T) { BlockFetcher: tvm.vm, SecpCache: tvm.vm.secpCache, } - - if err := exportTx.Visit(&atomic.SemanticVerifier{ - Backend: backend, - Tx: tx, - Parent: parent, - BaseFee: parent.ethBlock.BaseFee(), - }); err != nil { + if err := backend.SemanticVerify(tx, parent, parent.ethBlock.BaseFee()); err != nil { t.Fatal("newExportTx created an invalid transaction", err) } @@ -1995,13 +1981,7 @@ func TestNewExportTxMulticoin(t *testing.T) { BlockFetcher: tvm.vm, SecpCache: tvm.vm.secpCache, } - - if err := exportTx.Visit(&atomic.SemanticVerifier{ - Backend: backend, - Tx: tx, - Parent: parent, - BaseFee: parent.ethBlock.BaseFee(), - }); err != nil { + if err := backend.SemanticVerify(tx, parent, parent.ethBlock.BaseFee()); err != nil { t.Fatal("newExportTx created an invalid transaction", err) } diff --git a/plugin/evm/tx_test.go b/plugin/evm/tx_test.go index 6017f69205..ea96466c89 100644 --- a/plugin/evm/tx_test.go +++ b/plugin/evm/tx_test.go @@ -122,12 +122,7 @@ func executeTxTest(t *testing.T, test atomicTxTest) { BlockFetcher: tvm.vm, SecpCache: tvm.vm.secpCache, } - if err := tx.UnsignedAtomicTx.Visit(&atomic.SemanticVerifier{ - Backend: backend, - Tx: tx, - Parent: lastAcceptedBlock, - BaseFee: baseFee, - }); len(test.semanticVerifyErr) == 0 && err != nil { + if err := backend.SemanticVerify(tx, lastAcceptedBlock, baseFee); len(test.semanticVerifyErr) == 0 && err != nil { t.Fatalf("SemanticVerify failed unexpectedly due to: %s", err) } else if len(test.semanticVerifyErr) != 0 { if err == nil { diff --git a/plugin/evm/vm.go b/plugin/evm/vm.go index d1bdd21996..3973eea1e6 100644 --- a/plugin/evm/vm.go +++ b/plugin/evm/vm.go @@ -1602,12 +1602,7 @@ func (vm *VM) verifyTx(tx *atomic.Tx, parentHash common.Hash, baseFee *big.Int, BlockFetcher: vm, SecpCache: vm.secpCache, } - if err := tx.UnsignedAtomicTx.Visit(&atomic.SemanticVerifier{ - Backend: atomicBackend, - Tx: tx, - Parent: parent, - BaseFee: baseFee, - }); err != nil { + if atomicBackend.SemanticVerify(tx, parent, baseFee); err != nil { return err } return tx.UnsignedAtomicTx.EVMStateTransfer(vm.ctx, state) @@ -1646,17 +1641,11 @@ func (vm *VM) verifyTxs(txs []*atomic.Tx, parentHash common.Hash, baseFee *big.I BlockFetcher: vm, SecpCache: vm.secpCache, } - for _, atomicTx := range txs { - utx := atomicTx.UnsignedAtomicTx - if err := utx.Visit(&atomic.SemanticVerifier{ - Backend: atomicBackend, - Tx: atomicTx, - Parent: ancestor, - BaseFee: baseFee, - }); err != nil { - return fmt.Errorf("invalid block due to failed semanatic verify: %w at height %d", err, height) + for _, tx := range txs { + if atomicBackend.SemanticVerify(tx, ancestor, baseFee); err != nil { + return fmt.Errorf("invalid block due to failed semantic verify: %w at height %d", err, height) } - txInputs := utx.InputUTXOs() + txInputs := tx.UnsignedAtomicTx.InputUTXOs() if inputs.Overlaps(txInputs) { return atomic.ErrConflictingAtomicInputs } From b49e046e1749c671c522cc3498a4285be776df24 Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Tue, 27 May 2025 11:26:28 -0400 Subject: [PATCH 2/3] unexport fields --- plugin/evm/atomic/tx_semantic_verifier.go | 32 +++++++++++------------ 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/plugin/evm/atomic/tx_semantic_verifier.go b/plugin/evm/atomic/tx_semantic_verifier.go index 935eb42b89..8bb47df753 100644 --- a/plugin/evm/atomic/tx_semantic_verifier.go +++ b/plugin/evm/atomic/tx_semantic_verifier.go @@ -47,27 +47,27 @@ type VerifierBackend struct { // SemanticVerify checks the semantic validity of atomic transactions. func (b *VerifierBackend) SemanticVerify(tx *Tx, parent AtomicBlockContext, baseFee *big.Int) error { return tx.UnsignedAtomicTx.Visit(&semanticVerifier{ - Backend: b, - Tx: tx, - Parent: parent, - BaseFee: baseFee, + backend: b, + tx: tx, + parent: parent, + baseFee: baseFee, }) } // semanticVerifier is a visitor that checks the semantic validity of atomic // transactions. type semanticVerifier struct { - Backend *VerifierBackend - Tx *Tx - Parent AtomicBlockContext - BaseFee *big.Int + backend *VerifierBackend + tx *Tx + parent AtomicBlockContext + baseFee *big.Int } func (s *semanticVerifier) ImportTx(utx *UnsignedImportTx) error { - backend := s.Backend + backend := s.backend ctx := backend.Ctx rules := backend.Rules - stx := s.Tx + stx := s.tx if err := utx.Verify(ctx, rules); err != nil { return err } @@ -81,7 +81,7 @@ func (s *semanticVerifier) ImportTx(utx *UnsignedImportTx) error { if err != nil { return err } - txFee, err := CalculateDynamicFee(gasUsed, s.BaseFee) + txFee, err := CalculateDynamicFee(gasUsed, s.baseFee) if err != nil { return err } @@ -143,7 +143,7 @@ func (s *semanticVerifier) ImportTx(utx *UnsignedImportTx) error { } } - return conflicts(backend, utx.InputUTXOs(), s.Parent) + return conflicts(backend, utx.InputUTXOs(), s.parent) } // conflicts returns an error if [inputs] conflicts with any of the atomic inputs contained in [ancestor] @@ -187,10 +187,10 @@ func conflicts(backend *VerifierBackend, inputs set.Set[ids.ID], ancestor Atomic } func (s *semanticVerifier) ExportTx(utx *UnsignedExportTx) error { - backend := s.Backend + backend := s.backend ctx := backend.Ctx rules := backend.Rules - stx := s.Tx + stx := s.tx if err := utx.Verify(ctx, rules); err != nil { return err } @@ -204,7 +204,7 @@ func (s *semanticVerifier) ExportTx(utx *UnsignedExportTx) error { if err != nil { return err } - txFee, err := CalculateDynamicFee(gasUsed, s.BaseFee) + txFee, err := CalculateDynamicFee(gasUsed, s.baseFee) if err != nil { return err } @@ -240,7 +240,7 @@ func (s *semanticVerifier) ExportTx(utx *UnsignedExportTx) error { if len(cred.Sigs) != 1 { return fmt.Errorf("expected one signature for EVM Input Credential, but found: %d", len(cred.Sigs)) } - pubKey, err := s.Backend.SecpCache.RecoverPublicKey(utx.Bytes(), cred.Sigs[0][:]) + pubKey, err := s.backend.SecpCache.RecoverPublicKey(utx.Bytes(), cred.Sigs[0][:]) if err != nil { return err } From 6dcb286c96aa9f34c9bfdc9ae4263d8641298518 Mon Sep 17 00:00:00 2001 From: Stephen Buttolph Date: Tue, 27 May 2025 14:20:32 -0400 Subject: [PATCH 3/3] nit --- plugin/evm/export_tx_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/plugin/evm/export_tx_test.go b/plugin/evm/export_tx_test.go index 59bde7cb0b..06ee0d482e 100644 --- a/plugin/evm/export_tx_test.go +++ b/plugin/evm/export_tx_test.go @@ -1773,8 +1773,6 @@ func TestNewExportTx(t *testing.T) { t.Fatal(err) } - exportTx := tx.UnsignedAtomicTx - backend := &atomic.VerifierBackend{ Ctx: tvm.vm.ctx, Fx: &tvm.vm.fx, @@ -1787,6 +1785,7 @@ func TestNewExportTx(t *testing.T) { t.Fatal("newExportTx created an invalid transaction", err) } + exportTx := tx.UnsignedAtomicTx burnedAVAX, err := exportTx.Burned(tvm.vm.ctx.AVAXAssetID) if err != nil { t.Fatal(err) @@ -1972,7 +1971,6 @@ func TestNewExportTxMulticoin(t *testing.T) { t.Fatal(err) } - exportTx := tx.UnsignedAtomicTx backend := &atomic.VerifierBackend{ Ctx: tvm.vm.ctx, Fx: &tvm.vm.fx, @@ -1989,7 +1987,9 @@ func TestNewExportTxMulticoin(t *testing.T) { if err != nil { t.Fatalf("Failed to create commit batch for VM due to %s", err) } - chainID, atomicRequests, err := exportTx.AtomicOps() + + exportTx := tx.UnsignedAtomicTx + chainID, atomicRequests, err := tx.UnsignedAtomicTx.AtomicOps() if err != nil { t.Fatalf("Failed to accept export transaction due to: %s", err) }