From 9b676e8d9b8f489213f46b6efc0f1d7d168c1999 Mon Sep 17 00:00:00 2001 From: Aaron Buchwald Date: Tue, 10 Aug 2021 21:34:34 -0400 Subject: [PATCH 1/6] Add helper for copying address pointers and address TODO --- common/types.go | 10 ++++++++++ common/types_test.go | 23 +++++++++++++++++++++++ core/types/access_list_tx.go | 2 +- core/types/dynamic_fee_tx.go | 2 +- core/types/legacy_tx.go | 2 +- 5 files changed, 36 insertions(+), 3 deletions(-) diff --git a/common/types.go b/common/types.go index 2205835cb5d0..2233bca9ad2e 100644 --- a/common/types.go +++ b/common/types.go @@ -339,6 +339,16 @@ func (a Address) Value() (driver.Value, error) { return a[:], nil } +// CopyPointer returns a new pointer to a copy of the address or nil +// if [a] is nil. +func (a *Address) CopyPointer() *Address { + if a == nil { + return nil + } + cpy := *a + return &cpy +} + // ImplementsGraphQLType returns true if Hash implements the specified GraphQL type. func (a Address) ImplementsGraphQLType(name string) bool { return name == "Address" } diff --git a/common/types_test.go b/common/types_test.go index 318e985f870b..d753d54dd1c9 100644 --- a/common/types_test.go +++ b/common/types_test.go @@ -453,6 +453,29 @@ func TestAddress_Format(t *testing.T) { } } +func TestCopyAddress(t *testing.T) { + addrBytes := make([]byte, 20) + addr := BytesToAddress(addrBytes) + if !bytes.Equal(addr.Bytes(), addrBytes) { + t.Fatalf("Expected original address bytes to be unmodified") + } + addrPtr := &addr + cpy := addrPtr.CopyPointer() + cpy.SetBytes([]byte("deadbeef")) + if !bytes.Equal(addr.Bytes(), addrBytes) { + t.Fatal("Expected original address bytes to be unmodified") + } + if bytes.Equal(cpy.Bytes(), addrBytes) { + t.Fatal("Expected modified address to no longer match original byte slice") + } + + var nilAddr *Address + copiedNilAddr := nilAddr.CopyPointer() + if copiedNilAddr != nil { + t.Fatalf("Expected copied nil address to be nil, but got %s", copiedNilAddr) + } +} + func TestHash_Format(t *testing.T) { var hash Hash hash.SetBytes([]byte{ diff --git a/core/types/access_list_tx.go b/core/types/access_list_tx.go index d8d08b48c5bb..a4caab40ff31 100644 --- a/core/types/access_list_tx.go +++ b/core/types/access_list_tx.go @@ -59,7 +59,7 @@ type AccessListTx struct { func (tx *AccessListTx) copy() TxData { cpy := &AccessListTx{ Nonce: tx.Nonce, - To: tx.To, // TODO: copy pointed-to address + To: tx.To.CopyPointer(), Data: common.CopyBytes(tx.Data), Gas: tx.Gas, // These are copied below. diff --git a/core/types/dynamic_fee_tx.go b/core/types/dynamic_fee_tx.go index c6719a40898d..1d6d8a881185 100644 --- a/core/types/dynamic_fee_tx.go +++ b/core/types/dynamic_fee_tx.go @@ -43,7 +43,7 @@ type DynamicFeeTx struct { func (tx *DynamicFeeTx) copy() TxData { cpy := &DynamicFeeTx{ Nonce: tx.Nonce, - To: tx.To, // TODO: copy pointed-to address + To: tx.To.CopyPointer(), Data: common.CopyBytes(tx.Data), Gas: tx.Gas, // These are copied below. diff --git a/core/types/legacy_tx.go b/core/types/legacy_tx.go index 514010ebbd74..fc3700898c95 100644 --- a/core/types/legacy_tx.go +++ b/core/types/legacy_tx.go @@ -62,7 +62,7 @@ func NewContractCreation(nonce uint64, amount *big.Int, gasLimit uint64, gasPric func (tx *LegacyTx) copy() TxData { cpy := &LegacyTx{ Nonce: tx.Nonce, - To: tx.To, // TODO: copy pointed-to address + To: tx.To.CopyPointer(), Data: common.CopyBytes(tx.Data), Gas: tx.Gas, // These are initialized below. From 6f8876e92040333bd199ffa0146a68dcd00a3c61 Mon Sep 17 00:00:00 2001 From: Aaron Buchwald Date: Wed, 11 Aug 2021 16:48:07 -0400 Subject: [PATCH 2/6] Rename to Clone() --- common/types.go | 5 ++--- common/types_test.go | 4 ++-- core/types/access_list_tx.go | 2 +- core/types/dynamic_fee_tx.go | 2 +- core/types/legacy_tx.go | 2 +- 5 files changed, 7 insertions(+), 8 deletions(-) diff --git a/common/types.go b/common/types.go index 2233bca9ad2e..1b88d678d129 100644 --- a/common/types.go +++ b/common/types.go @@ -339,9 +339,8 @@ func (a Address) Value() (driver.Value, error) { return a[:], nil } -// CopyPointer returns a new pointer to a copy of the address or nil -// if [a] is nil. -func (a *Address) CopyPointer() *Address { +// Clone returns a pointer to a copy of [a] (or nil if [a] is nil) +func (a *Address) Clone() *Address { if a == nil { return nil } diff --git a/common/types_test.go b/common/types_test.go index d753d54dd1c9..e8243436dc37 100644 --- a/common/types_test.go +++ b/common/types_test.go @@ -460,7 +460,7 @@ func TestCopyAddress(t *testing.T) { t.Fatalf("Expected original address bytes to be unmodified") } addrPtr := &addr - cpy := addrPtr.CopyPointer() + cpy := addrPtr.Clone() cpy.SetBytes([]byte("deadbeef")) if !bytes.Equal(addr.Bytes(), addrBytes) { t.Fatal("Expected original address bytes to be unmodified") @@ -470,7 +470,7 @@ func TestCopyAddress(t *testing.T) { } var nilAddr *Address - copiedNilAddr := nilAddr.CopyPointer() + copiedNilAddr := nilAddr.Clone() if copiedNilAddr != nil { t.Fatalf("Expected copied nil address to be nil, but got %s", copiedNilAddr) } diff --git a/core/types/access_list_tx.go b/core/types/access_list_tx.go index a4caab40ff31..c5c9b6a73583 100644 --- a/core/types/access_list_tx.go +++ b/core/types/access_list_tx.go @@ -59,7 +59,7 @@ type AccessListTx struct { func (tx *AccessListTx) copy() TxData { cpy := &AccessListTx{ Nonce: tx.Nonce, - To: tx.To.CopyPointer(), + To: tx.To.Clone(), Data: common.CopyBytes(tx.Data), Gas: tx.Gas, // These are copied below. diff --git a/core/types/dynamic_fee_tx.go b/core/types/dynamic_fee_tx.go index 1d6d8a881185..f6688be16b24 100644 --- a/core/types/dynamic_fee_tx.go +++ b/core/types/dynamic_fee_tx.go @@ -43,7 +43,7 @@ type DynamicFeeTx struct { func (tx *DynamicFeeTx) copy() TxData { cpy := &DynamicFeeTx{ Nonce: tx.Nonce, - To: tx.To.CopyPointer(), + To: tx.To.Clone(), Data: common.CopyBytes(tx.Data), Gas: tx.Gas, // These are copied below. diff --git a/core/types/legacy_tx.go b/core/types/legacy_tx.go index fc3700898c95..9b95a9e902db 100644 --- a/core/types/legacy_tx.go +++ b/core/types/legacy_tx.go @@ -62,7 +62,7 @@ func NewContractCreation(nonce uint64, amount *big.Int, gasLimit uint64, gasPric func (tx *LegacyTx) copy() TxData { cpy := &LegacyTx{ Nonce: tx.Nonce, - To: tx.To.CopyPointer(), + To: tx.To.Clone(), Data: common.CopyBytes(tx.Data), Gas: tx.Gas, // These are initialized below. From 0b103bb87dcf32fd554748b41b0c2e099bcf7622 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Wed, 6 Oct 2021 12:10:54 +0200 Subject: [PATCH 3/6] core/types: add & use copyAddressPtr --- core/types/access_list_tx.go | 2 +- core/types/dynamic_fee_tx.go | 2 +- core/types/legacy_tx.go | 2 +- core/types/transaction.go | 17 ++++++++++------- 4 files changed, 13 insertions(+), 10 deletions(-) diff --git a/core/types/access_list_tx.go b/core/types/access_list_tx.go index c5c9b6a73583..d7539798e675 100644 --- a/core/types/access_list_tx.go +++ b/core/types/access_list_tx.go @@ -59,7 +59,7 @@ type AccessListTx struct { func (tx *AccessListTx) copy() TxData { cpy := &AccessListTx{ Nonce: tx.Nonce, - To: tx.To.Clone(), + To: copyAddressPtr(tx.To), Data: common.CopyBytes(tx.Data), Gas: tx.Gas, // These are copied below. diff --git a/core/types/dynamic_fee_tx.go b/core/types/dynamic_fee_tx.go index f6688be16b24..443b9c96879c 100644 --- a/core/types/dynamic_fee_tx.go +++ b/core/types/dynamic_fee_tx.go @@ -43,7 +43,7 @@ type DynamicFeeTx struct { func (tx *DynamicFeeTx) copy() TxData { cpy := &DynamicFeeTx{ Nonce: tx.Nonce, - To: tx.To.Clone(), + To: copyAddressPtr(tx.To), Data: common.CopyBytes(tx.Data), Gas: tx.Gas, // These are copied below. diff --git a/core/types/legacy_tx.go b/core/types/legacy_tx.go index 9b95a9e902db..cb86bed772bc 100644 --- a/core/types/legacy_tx.go +++ b/core/types/legacy_tx.go @@ -62,7 +62,7 @@ func NewContractCreation(nonce uint64, amount *big.Int, gasLimit uint64, gasPric func (tx *LegacyTx) copy() TxData { cpy := &LegacyTx{ Nonce: tx.Nonce, - To: tx.To.Clone(), + To: copyAddressPtr(tx.To), Data: common.CopyBytes(tx.Data), Gas: tx.Gas, // These are initialized below. diff --git a/core/types/transaction.go b/core/types/transaction.go index e21cf2bda826..51361e27d5ca 100644 --- a/core/types/transaction.go +++ b/core/types/transaction.go @@ -284,13 +284,7 @@ func (tx *Transaction) Nonce() uint64 { return tx.inner.nonce() } // To returns the recipient address of the transaction. // For contract-creation transactions, To returns nil. func (tx *Transaction) To() *common.Address { - // Copy the pointed-to address. - ito := tx.inner.to() - if ito == nil { - return nil - } - cpy := *ito - return &cpy + return copyAddressPtr(tx.inner.to()) } // Cost returns gas * gasPrice + value. @@ -632,3 +626,12 @@ func (m Message) Nonce() uint64 { return m.nonce } func (m Message) Data() []byte { return m.data } func (m Message) AccessList() AccessList { return m.accessList } func (m Message) IsFake() bool { return m.isFake } + +// copyAddressPtr copies an address. +func copyAddressPtr(a *common.Address) *common.Address { + if a == nil { + return nil + } + cpy := *a + return &coy +} From 939af885a250fe99cb91e3bc4e335dcb46442dcc Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Wed, 6 Oct 2021 12:11:46 +0200 Subject: [PATCH 4/6] common: remove Clone --- common/types.go | 9 --------- common/types_test.go | 23 ----------------------- 2 files changed, 32 deletions(-) diff --git a/common/types.go b/common/types.go index 1b88d678d129..2205835cb5d0 100644 --- a/common/types.go +++ b/common/types.go @@ -339,15 +339,6 @@ func (a Address) Value() (driver.Value, error) { return a[:], nil } -// Clone returns a pointer to a copy of [a] (or nil if [a] is nil) -func (a *Address) Clone() *Address { - if a == nil { - return nil - } - cpy := *a - return &cpy -} - // ImplementsGraphQLType returns true if Hash implements the specified GraphQL type. func (a Address) ImplementsGraphQLType(name string) bool { return name == "Address" } diff --git a/common/types_test.go b/common/types_test.go index e8243436dc37..318e985f870b 100644 --- a/common/types_test.go +++ b/common/types_test.go @@ -453,29 +453,6 @@ func TestAddress_Format(t *testing.T) { } } -func TestCopyAddress(t *testing.T) { - addrBytes := make([]byte, 20) - addr := BytesToAddress(addrBytes) - if !bytes.Equal(addr.Bytes(), addrBytes) { - t.Fatalf("Expected original address bytes to be unmodified") - } - addrPtr := &addr - cpy := addrPtr.Clone() - cpy.SetBytes([]byte("deadbeef")) - if !bytes.Equal(addr.Bytes(), addrBytes) { - t.Fatal("Expected original address bytes to be unmodified") - } - if bytes.Equal(cpy.Bytes(), addrBytes) { - t.Fatal("Expected modified address to no longer match original byte slice") - } - - var nilAddr *Address - copiedNilAddr := nilAddr.Clone() - if copiedNilAddr != nil { - t.Fatalf("Expected copied nil address to be nil, but got %s", copiedNilAddr) - } -} - func TestHash_Format(t *testing.T) { var hash Hash hash.SetBytes([]byte{ From 7ece964d3896e81b6a4d23942a0983d15475a2e6 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Wed, 6 Oct 2021 12:13:16 +0200 Subject: [PATCH 5/6] core/types: fix typo --- core/types/transaction.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/types/transaction.go b/core/types/transaction.go index 51361e27d5ca..83f1766e67e2 100644 --- a/core/types/transaction.go +++ b/core/types/transaction.go @@ -633,5 +633,5 @@ func copyAddressPtr(a *common.Address) *common.Address { return nil } cpy := *a - return &coy + return &cpy } From d264447f0e3cea173cc093ec8d828b33f0b5de05 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Wed, 6 Oct 2021 12:14:40 +0200 Subject: [PATCH 6/6] core/types: remove unused method protected() --- core/types/access_list_tx.go | 1 - core/types/dynamic_fee_tx.go | 1 - 2 files changed, 2 deletions(-) diff --git a/core/types/access_list_tx.go b/core/types/access_list_tx.go index d7539798e675..ee5f194b77b8 100644 --- a/core/types/access_list_tx.go +++ b/core/types/access_list_tx.go @@ -96,7 +96,6 @@ func (tx *AccessListTx) copy() TxData { // accessors for innerTx. func (tx *AccessListTx) txType() byte { return AccessListTxType } func (tx *AccessListTx) chainID() *big.Int { return tx.ChainID } -func (tx *AccessListTx) protected() bool { return true } func (tx *AccessListTx) accessList() AccessList { return tx.AccessList } func (tx *AccessListTx) data() []byte { return tx.Data } func (tx *AccessListTx) gas() uint64 { return tx.Gas } diff --git a/core/types/dynamic_fee_tx.go b/core/types/dynamic_fee_tx.go index 443b9c96879c..585c029d8901 100644 --- a/core/types/dynamic_fee_tx.go +++ b/core/types/dynamic_fee_tx.go @@ -84,7 +84,6 @@ func (tx *DynamicFeeTx) copy() TxData { // accessors for innerTx. func (tx *DynamicFeeTx) txType() byte { return DynamicFeeTxType } func (tx *DynamicFeeTx) chainID() *big.Int { return tx.ChainID } -func (tx *DynamicFeeTx) protected() bool { return true } func (tx *DynamicFeeTx) accessList() AccessList { return tx.AccessList } func (tx *DynamicFeeTx) data() []byte { return tx.Data } func (tx *DynamicFeeTx) gas() uint64 { return tx.Gas }