Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CIP-64 / CIP-66 compatible TransactionArgs #123

Merged
merged 11 commits into from
Jun 12, 2024
2 changes: 1 addition & 1 deletion accounts/external/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ func (api *ExternalSigner) SignTx(account accounts.Account, tx *types.Transactio
switch tx.Type() {
case types.LegacyTxType, types.AccessListTxType:
args.GasPrice = (*hexutil.Big)(tx.GasPrice())
case types.DynamicFeeTxType, types.CeloDynamicFeeTxType:
case types.DynamicFeeTxType, types.CeloDynamicFeeTxType, types.CeloDenominatedTxType:
args.MaxFeePerGas = (*hexutil.Big)(tx.GasFeeCap())
args.MaxPriorityFeePerGas = (*hexutil.Big)(tx.GasTipCap())
default:
Expand Down
5 changes: 2 additions & 3 deletions core/state_transition.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,7 @@ type Message struct {
// FeeCurrency specifies the currency for gas fees.
// `nil` corresponds to Celo Gold (native currency).
// All other values should correspond to ERC20 contract addresses.
FeeCurrency *common.Address

FeeCurrency *common.Address
MaxFeeInFeeCurrency *big.Int // MaxFeeInFeeCurrency is the maximum fee that can be charged in the fee currency.
}

Expand Down Expand Up @@ -213,7 +212,7 @@ func TransactionToMessage(tx *types.Transaction, s types.Signer, baseFee *big.In
}
// If baseFee provided, set gasPrice to effectiveGasPrice.
if baseFee != nil {
if msg.FeeCurrency != nil {
if tx.Type() == types.CeloDynamicFeeTxType {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change safe for the case where tx.Type() == types.CeloDynamicFeeTxType but msg.FeeCurrency == nil? Unfortunately, we didn't forbid nil currencies for CIP-64.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the ConvertGoldToCurrency function just returns the passed in value in that case (here baseFee, which has the same effect as omitting the if case).

var err error
baseFee, err = exchange.ConvertGoldToCurrency(exchangeRates, msg.FeeCurrency, baseFee)
if err != nil {
Expand Down
121 changes: 121 additions & 0 deletions core/types/celo_denominated_tx.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
package types

import (
"bytes"
"math/big"

"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/rlp"
)

type CeloDenominatedTx struct {
ChainID *big.Int
Nonce uint64
GasTipCap *big.Int
GasFeeCap *big.Int
Gas uint64
To *common.Address `rlp:"nil"` // nil means contract creation
Value *big.Int
Data []byte
AccessList AccessList

FeeCurrency *common.Address
MaxFeeInFeeCurrency *big.Int

// Signature values
V *big.Int `json:"v" gencodec:"required"`
R *big.Int `json:"r" gencodec:"required"`
S *big.Int `json:"s" gencodec:"required"`
}

// copy creates a deep copy of the transaction data and initializes all fields.
func (tx *CeloDenominatedTx) copy() TxData {
cpy := &CeloDenominatedTx{
Nonce: tx.Nonce,
To: copyAddressPtr(tx.To),
Data: common.CopyBytes(tx.Data),
Gas: tx.Gas,
FeeCurrency: copyAddressPtr(tx.FeeCurrency),
// These are copied below.
MaxFeeInFeeCurrency: new(big.Int),
AccessList: make(AccessList, len(tx.AccessList)),
Value: new(big.Int),
ChainID: new(big.Int),
GasTipCap: new(big.Int),
GasFeeCap: new(big.Int),
V: new(big.Int),
R: new(big.Int),
S: new(big.Int),
}
if tx.MaxFeeInFeeCurrency != nil {
cpy.MaxFeeInFeeCurrency.Set(tx.MaxFeeInFeeCurrency)
}
copy(cpy.AccessList, tx.AccessList)
if tx.Value != nil {
cpy.Value.Set(tx.Value)
}
if tx.ChainID != nil {
cpy.ChainID.Set(tx.ChainID)
}
if tx.GasTipCap != nil {
cpy.GasTipCap.Set(tx.GasTipCap)
}
if tx.GasFeeCap != nil {
cpy.GasFeeCap.Set(tx.GasFeeCap)
}
if tx.V != nil {
cpy.V.Set(tx.V)
}
if tx.R != nil {
cpy.R.Set(tx.R)
}
if tx.S != nil {
cpy.S.Set(tx.S)
}
return cpy
}

// accessors for innerTx.
func (tx *CeloDenominatedTx) txType() byte { return CeloDenominatedTxType }
func (tx *CeloDenominatedTx) chainID() *big.Int { return tx.ChainID }
func (tx *CeloDenominatedTx) accessList() AccessList { return tx.AccessList }
func (tx *CeloDenominatedTx) data() []byte { return tx.Data }
func (tx *CeloDenominatedTx) gas() uint64 { return tx.Gas }
func (tx *CeloDenominatedTx) gasFeeCap() *big.Int { return tx.GasFeeCap }
func (tx *CeloDenominatedTx) gasTipCap() *big.Int { return tx.GasTipCap }
func (tx *CeloDenominatedTx) gasPrice() *big.Int { return tx.GasFeeCap }
func (tx *CeloDenominatedTx) value() *big.Int { return tx.Value }
func (tx *CeloDenominatedTx) nonce() uint64 { return tx.Nonce }
func (tx *CeloDenominatedTx) to() *common.Address { return tx.To }
func (tx *CeloDenominatedTx) isSystemTx() bool { return false }

func (tx *CeloDenominatedTx) effectiveGasPrice(dst *big.Int, baseFee *big.Int) *big.Int {
if baseFee == nil {
return dst.Set(tx.GasFeeCap)
}
tip := dst.Sub(tx.GasFeeCap, baseFee)
if tip.Cmp(tx.GasTipCap) > 0 {
tip.Set(tx.GasTipCap)
}
return tip.Add(tip, baseFee)
}

func (tx *CeloDenominatedTx) rawSignatureValues() (v, r, s *big.Int) {
return tx.V, tx.R, tx.S
}

func (tx *CeloDenominatedTx) setSignatureValues(chainID, v, r, s *big.Int) {
tx.ChainID, tx.V, tx.R, tx.S = chainID, v, r, s
}

func (tx *CeloDenominatedTx) encode(b *bytes.Buffer) error {
return rlp.Encode(b, tx)
}

func (tx *CeloDenominatedTx) decode(input []byte) error {
return rlp.DecodeBytes(input, tx)
}

func (tx *CeloDenominatedTx) feeCurrency() *common.Address { return tx.FeeCurrency }

func (tx *CeloDenominatedTx) maxFeeInFeeCurrency() *big.Int { return tx.MaxFeeInFeeCurrency }
3 changes: 2 additions & 1 deletion core/types/celo_dynamic_fee_tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,4 +114,5 @@ func (tx *CeloDynamicFeeTx) decode(input []byte) error {
return rlp.DecodeBytes(input, tx)
}

func (tx *CeloDynamicFeeTx) feeCurrency() *common.Address { return tx.FeeCurrency }
func (tx *CeloDynamicFeeTx) feeCurrency() *common.Address { return tx.FeeCurrency }
func (tx *CeloDynamicFeeTx) maxFeeInFeeCurrency() *big.Int { return nil }
26 changes: 22 additions & 4 deletions core/types/celo_transaction_signing.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func NewCel2Signer(chainId *big.Int) Signer {
}

func (s cel2Signer) Sender(tx *Transaction) (common.Address, error) {
if tx.Type() != CeloDynamicFeeTxType {
if tx.Type() != CeloDynamicFeeTxType && tx.Type() != CeloDenominatedTxType {
return s.londonSigner.Sender(tx)
}
V, R, S := tx.RawSignatureValues()
Expand All @@ -55,13 +55,14 @@ func (s cel2Signer) Equal(s2 Signer) bool {
}

func (s cel2Signer) SignatureValues(tx *Transaction, sig []byte) (R, S, V *big.Int, err error) {
txdata, ok := tx.inner.(*CeloDynamicFeeTx)
if !ok {
if tx.Type() != CeloDynamicFeeTxType && tx.Type() != CeloDenominatedTxType {
return s.londonSigner.SignatureValues(tx, sig)
}

// Check that chain ID of tx matches the signer. We also accept ID zero here,
// because it indicates that the chain ID was not specified in the tx.
if txdata.ChainID.Sign() != 0 && txdata.ChainID.Cmp(s.chainId) != 0 {
chainID := tx.inner.chainID()
if chainID.Sign() != 0 && chainID.Cmp(s.chainId) != 0 {
return nil, nil, nil, ErrInvalidChainId
}
R, S, _ = decodeSignature(sig)
Expand All @@ -88,5 +89,22 @@ func (s cel2Signer) Hash(tx *Transaction) common.Hash {
tx.FeeCurrency(),
})
}
if tx.Type() == CeloDenominatedTxType {
return prefixedRlpHash(
tx.Type(),
[]interface{}{
s.chainId,
tx.Nonce(),
tx.GasTipCap(),
tx.GasFeeCap(),
tx.Gas(),
tx.To(),
tx.Value(),
tx.Data(),
tx.AccessList(),
tx.FeeCurrency(),
tx.MaxFeeInFeeCurrency(),
})
}
return s.londonSigner.Hash(tx)
}
3 changes: 2 additions & 1 deletion core/types/deposit_tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,4 +102,5 @@ func (tx *DepositTx) decode(input []byte) error {
return rlp.DecodeBytes(input, tx)
}

func (tx *DepositTx) feeCurrency() *common.Address { return nil }
func (tx *DepositTx) feeCurrency() *common.Address { return nil }
func (tx *DepositTx) maxFeeInFeeCurrency() *big.Int { return nil }
16 changes: 15 additions & 1 deletion core/types/transaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ const (
DynamicFeeTxType = 0x02
BlobTxType = 0x03
// CeloDynamicFeeTxType = 0x7c old Celo tx type with gateway fee
CeloDynamicFeeTxType = 0x7b
CeloDynamicFeeTxType = 0x7b
CeloDenominatedTxType = 0x7a
)

// Transaction is an Ethereum transaction.
Expand Down Expand Up @@ -110,6 +111,7 @@ type TxData interface {

// Celo specific fields
feeCurrency() *common.Address
maxFeeInFeeCurrency() *big.Int
Comment on lines 113 to +114
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC @piersy suggested that we can avoid extending TxData by checking the tx type and casting to a specific tx type whenever we want to use a tx-type-specific attribute. I think that can work, but for consistency it should either be done for feeCurrency too or not at all.
I think going with the current approach is fine for now. I just wanted to bring up the point to see if there are any strong opionions or other thoughs about it.

Copy link
Author

@ezdac ezdac Jun 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I generally don't like the concept of the TxData interface exposing the union of all struct fields.
If I would contribute to upstream, I would vote to keep the changes of this PR as is for the purpose of consistency though.
When considering minimizing the diff and stronger segregation of Celo specific features for our fork I tend to lean towards the suggestion of @piersy.

So do you suggest implementing this in this PR?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, let's keep it as it is. If we want to change it, we should do so in a separate PR.

}

// EncodeRLP implements rlp.Encoder
Expand Down Expand Up @@ -216,6 +218,8 @@ func (tx *Transaction) decodeTyped(b []byte) (TxData, error) {
inner = new(DynamicFeeTx)
case CeloDynamicFeeTxType:
inner = new(CeloDynamicFeeTx)
case CeloDenominatedTxType:
inner = new(CeloDenominatedTx)
case BlobTxType:
inner = new(BlobTx)
case DepositTxType:
Expand Down Expand Up @@ -628,6 +632,16 @@ func (tx *Transaction) FeeCurrency() *common.Address {
return copyAddressPtr(tx.inner.feeCurrency())
}

// MaxFeeInFeeCurrency is only used to guard against very quickly changing exchange rates.
// Txs must be discarded if MaxFeeInFeeCurrency is exceeded.
func (tx *Transaction) MaxFeeInFeeCurrency() *big.Int {
mfifc := tx.inner.maxFeeInFeeCurrency()
if mfifc == nil {
return nil
}
return new(big.Int).Set(mfifc)
}

// Transactions implements DerivableList for transactions.
type Transactions []*Transaction

Expand Down
3 changes: 2 additions & 1 deletion core/types/tx_access_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,4 +129,5 @@ func (tx *AccessListTx) decode(input []byte) error {
return rlp.DecodeBytes(input, tx)
}

func (tx *AccessListTx) feeCurrency() *common.Address { return nil }
func (tx *AccessListTx) feeCurrency() *common.Address { return nil }
func (tx *AccessListTx) maxFeeInFeeCurrency() *big.Int { return nil }
3 changes: 2 additions & 1 deletion core/types/tx_blob.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,4 +238,5 @@ func (tx *BlobTx) decode(input []byte) error {
return nil
}

func (tx *BlobTx) feeCurrency() *common.Address { return nil }
func (tx *BlobTx) feeCurrency() *common.Address { return nil }
func (tx *BlobTx) maxFeeInFeeCurrency() *big.Int { return nil }
3 changes: 2 additions & 1 deletion core/types/tx_dynamic_fee.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,4 +125,5 @@ func (tx *DynamicFeeTx) decode(input []byte) error {
return rlp.DecodeBytes(input, tx)
}

func (tx *DynamicFeeTx) feeCurrency() *common.Address { return nil }
func (tx *DynamicFeeTx) feeCurrency() *common.Address { return nil }
func (tx *DynamicFeeTx) maxFeeInFeeCurrency() *big.Int { return nil }
3 changes: 2 additions & 1 deletion core/types/tx_legacy.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,4 +125,5 @@ func (tx *LegacyTx) decode([]byte) error {
panic("decode called on LegacyTx)")
}

func (tx *LegacyTx) feeCurrency() *common.Address { return nil }
func (tx *LegacyTx) feeCurrency() *common.Address { return nil }
func (tx *LegacyTx) maxFeeInFeeCurrency() *big.Int { return nil }
8 changes: 4 additions & 4 deletions internal/celoapi/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,18 +60,18 @@ func (b *CeloAPIBackend) GetExchangeRates(ctx context.Context, blockNumOrHash rp
return er, nil
}

func (b *CeloAPIBackend) ConvertToCurrency(ctx context.Context, blockNumOrHash rpc.BlockNumberOrHash, value *big.Int, fromFeeCurrency *common.Address) (*big.Int, error) {
func (b *CeloAPIBackend) ConvertToCurrency(ctx context.Context, blockNumOrHash rpc.BlockNumberOrHash, goldAmount *big.Int, toFeeCurrency *common.Address) (*big.Int, error) {
er, err := b.GetExchangeRates(ctx, blockNumOrHash)
if err != nil {
return nil, err
}
return exchange.ConvertGoldToCurrency(er, fromFeeCurrency, value)
return exchange.ConvertGoldToCurrency(er, toFeeCurrency, goldAmount)
}

func (b *CeloAPIBackend) ConvertToGold(ctx context.Context, blockNumOrHash rpc.BlockNumberOrHash, value *big.Int, toFeeCurrency *common.Address) (*big.Int, error) {
func (b *CeloAPIBackend) ConvertToGold(ctx context.Context, blockNumOrHash rpc.BlockNumberOrHash, currencyAmount *big.Int, fromFeeCurrency *common.Address) (*big.Int, error) {
er, err := b.GetExchangeRates(ctx, blockNumOrHash)
if err != nil {
return nil, err
}
return exchange.ConvertCurrencyToGold(er, value, toFeeCurrency)
return exchange.ConvertCurrencyToGold(er, currencyAmount, fromFeeCurrency)
}
8 changes: 5 additions & 3 deletions internal/ethapi/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -1477,7 +1477,8 @@ type RPCTransaction struct {
DepositReceiptVersion *hexutil.Uint64 `json:"depositReceiptVersion,omitempty"`

// Celo
FeeCurrency *common.Address `json:"feeCurrency,omitempty"`
FeeCurrency *common.Address `json:"feeCurrency,omitempty"`
MaxFeeInFeeCurrency *hexutil.Big `json:"maxFeeInFeeCurrency,omitempty"`
}

// newRPCTransaction returns a transaction that will serialize to the RPC
Expand All @@ -1500,7 +1501,8 @@ func newRPCTransaction(tx *types.Transaction, blockHash common.Hash, blockNumber
R: (*hexutil.Big)(r),
S: (*hexutil.Big)(s),
// Celo
FeeCurrency: tx.FeeCurrency(),
FeeCurrency: tx.FeeCurrency(),
MaxFeeInFeeCurrency: (*hexutil.Big)(tx.MaxFeeInFeeCurrency()),
}
if blockHash != (common.Hash{}) {
result.BlockHash = &blockHash
Expand Down Expand Up @@ -1542,7 +1544,7 @@ func newRPCTransaction(tx *types.Transaction, blockHash common.Hash, blockNumber
result.ChainID = (*hexutil.Big)(tx.ChainId())
result.YParity = &yparity

case types.DynamicFeeTxType, types.CeloDynamicFeeTxType:
case types.DynamicFeeTxType, types.CeloDynamicFeeTxType, types.CeloDenominatedTxType:
al := tx.AccessList()
yparity := hexutil.Uint64(v.Sign())
result.Accesses = &al
Expand Down
Loading