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.

Sign up for GitHub

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

avoid evm.call for SGT #10

Merged
merged 3 commits into from
Oct 12, 2024
Merged

avoid evm.call for SGT #10

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a662019
avoid evm.call for SGT
blockchaindevsh Oct 9, 2024
0a47c98
address comments
blockchaindevsh Oct 11, 2024
850f1c9
export BalancesSlot for external testability
blockchaindevsh Oct 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
147 changes: 36 additions & 111 deletions core/state_transition.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ import (
"math/big"

"github.com/ethereum/go-ethereum/accounts/abi"
"github.com/ethereum/go-ethereum/accounts/abi/util"
"github.com/ethereum/go-ethereum/common"
cmath "github.com/ethereum/go-ethereum/common/math"
"github.com/ethereum/go-ethereum/core/types"
"github.com/ethereum/go-ethereum/core/vm"
"github.com/ethereum/go-ethereum/crypto"
"github.com/ethereum/go-ethereum/crypto/kzg4844"
"github.com/ethereum/go-ethereum/params"
"github.com/holiman/uint256"
Expand Down Expand Up @@ -245,127 +245,55 @@ func (st *StateTransition) to() common.Address {
return *st.msg.To
}

var SoulGasTokenABI abi.ABI

func init() {
var err error
SoulGasTokenABI, err = util.ParseFunctionsAsABI([]string{
"function deposit()", // used by op_geth_test.go
"function balanceOf(address account) returns (uint256 balance)",
"function burnFrom(address account, uint256 value)",
"function batchMint(address[] accounts, uint256[] values)"})
if err != nil {
panic(err)
}
}

func getSoulBalanceData(account common.Address) []byte {
method, ok := SoulGasTokenABI.Methods["balanceOf"]
if !ok {
panic("balanceOf method not found")
}

argument, err := method.Inputs.Pack(account)
if err != nil {
panic("failed to pack argument")
}

data := make([]byte, len(method.ID)+len(argument))
copy(data[0:len(method.ID)], method.ID)
copy(data[len(method.ID):], argument)
return data
}

func parseSoulBalanceResp(ret []byte) (*uint256.Int, error) {
method, ok := SoulGasTokenABI.Methods["balanceOf"]
if !ok {
panic("balanceOf method not found")
}
const (
// should keep it in sync with the balances field of SoulGasToken contract
balancesSlot = uint64(51)
Copy link

Choose a reason for hiding this comment

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

I am thinking whether we can get the slot by an external call as

 function getSlot() public pure returns (uint256) {
        uint256 slot;
        assembly {
            slot := _balances.slot
        }
        return slot;
    }

but definitively, it requires a lot of changes of code

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Gotcha, I think we can add a test in the monorepo that automatically checks for this.

)

returnValueInterface, err := method.Outputs.Unpack(ret)
if err != nil {
return nil, fmt.Errorf("parseSoulBalanceResp Unpack failed:%w", err)
}
var (
slotArgs abi.Arguments
)

returnValue := new(big.Int)
err = method.Outputs.Copy(&returnValue, returnValueInterface)
if err != nil {
return nil, fmt.Errorf("parseSoulBalanceResp Copy failed:%w", err)
}
returnValueU256, overflow := uint256.FromBig(returnValue)
if overflow {
return nil, fmt.Errorf("parsed soul balance overflow:%v", returnValue)
}
return returnValueU256, nil
func init() {
uint64Ty, _ := abi.NewType("uint64", "", nil)
addressTy, _ := abi.NewType("address", "", nil)
slotArgs = abi.Arguments{{Name: "addr", Type: addressTy, Indexed: false}, {Name: "slot", Type: uint64Ty, Indexed: false}}
}

func burnSoulBalanceData(account common.Address, amount *big.Int) []byte {

method, ok := SoulGasTokenABI.Methods["burnFrom"]
if !ok {
panic("burnFrom method not found")
}

argument, err := method.Inputs.Pack(account, amount)
if err != nil {
panic("failed to pack argument")
}

data := make([]byte, len(method.ID)+len(argument))
copy(data[0:len(method.ID)], method.ID)
copy(data[len(method.ID):], argument)
return data
func targetSlot(account common.Address) (slot common.Hash) {
data, _ := slotArgs.Pack(account, balancesSlot)
slot = crypto.Keccak256Hash(data)
return
}

func mintSoulBalanceData(account common.Address, amount *big.Int) []byte {
method, ok := SoulGasTokenABI.Methods["batchMint"]
if !ok {
panic("batchMint method not found")
}

argument, err := method.Inputs.Pack([]common.Address{account}, []*big.Int{amount})
if err != nil {
panic("failed to pack argument")
}

data := make([]byte, len(method.ID)+len(argument))
copy(data[0:len(method.ID)], method.ID)
copy(data[len(method.ID):], argument)
return data
func (st *StateTransition) GetSoulBalance(account common.Address) *uint256.Int {
slot := targetSlot(account)
value := st.state.GetState(types.SoulGasTokenAddr, slot)
balance := new(uint256.Int)
balance.SetBytes(value[:])
return balance
}

var (
callSoulGasLimit = uint64(8_000_000)
// use hardcoded DEPOSITOR_ACCOUNT both as minter and burner
depositorAddress = common.HexToAddress("0xDeaDDEaDDeAdDeAdDEAdDEaddeAddEAdDEAd0001")
)

func (st *StateTransition) GetSoulBalance(account common.Address) (*uint256.Int, error) {
// this evm call is free of gas charging
ret, _, vmerr := st.evm.StaticCall(vm.AccountRef(account), types.SoulGasTokenAddr, getSoulBalanceData(account), callSoulGasLimit)
if vmerr != nil {
return nil, vmerr
func (st *StateTransition) SubSoulBalance(account common.Address, amount *big.Int) (err error) {
current := st.GetSoulBalance(account).ToBig()
if current.Cmp(amount) < 0 {
return fmt.Errorf("soul balance not enough, current:%v, expect:%v", current, amount)
}
return parseSoulBalanceResp(ret)
}
var value common.Hash
current.Sub(current, amount).FillBytes(value[:])
Copy link

Choose a reason for hiding this comment

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

Can we use uint256.MustFromBig(current.Sub(current, amount)).Bytes32()? This will address the uncertainty of the endianness issue.

Copy link
Collaborator Author

@blockchaindevsh blockchaindevsh Oct 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Great suggestion! Done, and the test also changed to Bytes32().

st.state.SetState(types.SoulGasTokenAddr, targetSlot(account), value)

func (st *StateTransition) SubSoulBalance(account common.Address, amount *big.Int) (err error) {
_, _, err = st.evm.Call(vm.AccountRef(depositorAddress), types.SoulGasTokenAddr, burnSoulBalanceData(account, amount), callSoulGasLimit, common.U2560)
if err == nil {
if st.evm.ChainConfig().IsOptimism() && st.evm.ChainConfig().Optimism.IsSoulBackedByNative {
st.state.SubBalance(types.SoulGasTokenAddr, uint256.MustFromBig(amount))
}
if st.evm.ChainConfig().IsOptimism() && st.evm.ChainConfig().Optimism.IsSoulBackedByNative {
st.state.SubBalance(types.SoulGasTokenAddr, uint256.MustFromBig(amount))
}
return
}

func (st *StateTransition) AddSoulBalance(account common.Address, amount *big.Int) {

_, _, err := st.evm.Call(vm.AccountRef(depositorAddress), types.SoulGasTokenAddr, mintSoulBalanceData(account, amount), callSoulGasLimit, common.U2560)

if err != nil {
panic(fmt.Sprintf("mint should never fail:%v", err))
}
current := st.GetSoulBalance(account).ToBig()
var value common.Hash
current.Add(current, amount).FillBytes(value[:])
st.state.SetState(types.SoulGasTokenAddr, targetSlot(account), value)

if st.evm.ChainConfig().IsOptimism() && st.evm.ChainConfig().Optimism.IsSoulBackedByNative {
st.state.AddBalance(types.SoulGasTokenAddr, uint256.MustFromBig(amount))
Expand Down Expand Up @@ -413,10 +341,7 @@ func (st *StateTransition) buyGas() error {

// SoulGasToken doesn't support burning balance of zero account, so here we ensure that `gasFromSoul` is false for zero account.
if st.msg.From != (common.Address{}) && st.evm.ChainConfig().IsOptimism() && st.evm.ChainConfig().Optimism.UseSoulGasToken {
have, err := st.GetSoulBalance(st.msg.From)
if err != nil {
return fmt.Errorf("GetSoulBalance error:%v", err)
}
have := st.GetSoulBalance(st.msg.From)
if have, want := have.ToBig(), new(big.Int).Sub(balanceCheck, st.msg.Value); have.Cmp(want) >= 0 {
if have, want := st.state.GetBalance(st.msg.From).ToBig(), st.msg.Value; have.Cmp(want) < 0 {
Copy link

Choose a reason for hiding this comment

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

Does this mean that even if the sender has enough soul gas, it will still return an error if there isn’t enough native gas token?

Copy link
Collaborator Author

@blockchaindevsh blockchaindevsh Oct 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

No, that issue is to be fixed by modifying ValidateTransactionWithState to also consider SGT balance. I'll create a separate PR after merging this one.

Copy link

Choose a reason for hiding this comment

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

In what situation that the msg is from 0x0 address?

Copy link
Collaborator Author

@blockchaindevsh blockchaindevsh Oct 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

When called from here or here. Basically those APIs that doesn't require signature, in that case user can specify arbitrarily.

return fmt.Errorf("%w: address %v have %v want %v", ErrInsufficientFunds, st.msg.From.Hex(), have, want)
Expand Down