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

Jump to bottom

account type with zero init nonce #3387

Merged
merged 12 commits into from
Jul 2, 2022
Merged

account type with zero init nonce #3387

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
99f3ffb
set balance via SubBalance and AddBalance
CoderZhi May 20, 2022
8598056
create account with NewAccount and NewEmptyAccount
CoderZhi May 20, 2022
478d264
refactor state.nonce
CoderZhi May 22, 2022
b397bcb
skip update nonce and gas for system actions
CoderZhi May 23, 2022
878c583
check nonce value
CoderZhi May 20, 2022
2bae970
account type
CoderZhi May 13, 2022
524115e
read state with context
CoderZhi Jun 15, 2022
79aeaee
address comments
CoderZhi Jun 27, 2022
59509d1
fix unit test
CoderZhi Jun 28, 2022
877cf1a
address comment
CoderZhi Jun 28, 2022
50c36f9
address comment
CoderZhi Jul 1, 2022
fe5c397
Merge branch 'master' into nonce
Liuhaai Jul 1, 2022
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
1 change: 1 addition & 0 deletions action/const.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ var (
ErrChainID = errors.New("invalid chainID")
ErrExistedInPool = errors.New("known transaction")
ErrReplaceUnderpriced = errors.New("replacement transaction underpriced")
ErrSystemActionNonce = errors.New("invalid system action nonce")
ErrNonceTooLow = errors.New("nonce too low")
ErrUnderpriced = errors.New("transaction underpriced")
ErrNegativeValue = errors.New("negative value")
Expand Down
104 changes: 86 additions & 18 deletions action/protocol/account/accountpb/account.pb.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 8 additions & 2 deletions action/protocol/account/accountpb/account.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2018 IoTeX
// Copyright (c) 2022 IoTeX
// This is an alpha (internal) release and is not suitable for production. This source code is provided 'as is' and no
// warranties are given as to title or non-infringement, merchantability or fitness for purpose and, to the extent
// permitted by law, all liability for your use of the code is disclaimed. This source code is governed by Apache
Expand All @@ -8,6 +8,12 @@
// protoc --go_out=plugins=grpc:. *.proto
syntax = "proto3";
package accountpb;
option go_package = "github.com/iotexproject/iotex-core/action/protocol/account/accountpb";

enum AccountType {
DEFAULT = 0;
ZERO_NONCE = 1;
}

message Account {
// used by state-based model
Expand All @@ -17,5 +23,5 @@ message Account {
bytes codeHash = 4;
bool isCandidate = 5;
bytes votingWeight = 6;
AccountType type = 7;
}

21 changes: 15 additions & 6 deletions action/protocol/account/protocol.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,20 +107,25 @@ func (p *Protocol) Name() string {
return protocolID
}

func createAccount(sm protocol.StateManager, encodedAddr string, init *big.Int) error {
var account state.Account
func createAccount(sm protocol.StateManager, encodedAddr string, init *big.Int, opts ...state.AccountCreationOption) error {
account := &state.Account{}
addr, err := address.FromString(encodedAddr)
if err != nil {
return errors.Wrap(err, "failed to get address public key hash from encoded address")
}
addrHash := hash.BytesToHash160(addr.Bytes())
_, err = sm.State(&account, protocol.LegacyKeyOption(addrHash))
_, err = sm.State(account, protocol.LegacyKeyOption(addrHash))
switch errors.Cause(err) {
case nil:
return errors.Errorf("failed to create account %s", encodedAddr)
case state.ErrStateNotExist:
account.Balance = init
account.VotingWeight = big.NewInt(0)
account, err := state.NewAccount(opts...)
if err != nil {
return err
}
if err := account.AddBalance(init); err != nil {
return errors.Wrapf(err, "failed to add balance %s", init)
}
if _, err := sm.PutState(account, protocol.LegacyKeyOption(addrHash)); err != nil {
return errors.Wrapf(err, "failed to put state for account %x", addrHash)
}
Expand All @@ -143,8 +148,12 @@ func (p *Protocol) CreateGenesisStates(ctx context.Context, sm protocol.StateMan
if err := p.assertAmounts(amounts); err != nil {
return err
}
opts := []state.AccountCreationOption{}
if protocol.MustGetFeatureCtx(ctx).CreateLegacyNonceAccount {
opts = append(opts, state.LegacyNonceAccountTypeOption())
}
for i, addr := range addrs {
if err := createAccount(sm, addr.String(), amounts[i]); err != nil {
if err := createAccount(sm, addr.String(), amounts[i], opts...); err != nil {
return err
}
}
Expand Down
8 changes: 4 additions & 4 deletions action/protocol/account/protocol_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,14 @@ func TestLoadOrCreateAccountState(t *testing.T) {
addrv1 := identityset.Address(27)
s, err := accountutil.LoadAccount(sm, addrv1)
require.NoError(err)
require.Equal(s.Balance, state.EmptyAccount().Balance)
require.Equal(s.Balance, big.NewInt(0))

// create account
require.NoError(createAccount(sm, addrv1.String(), big.NewInt(5)))
require.NoError(createAccount(sm, addrv1.String(), big.NewInt(5), state.LegacyNonceAccountTypeOption()))
s, err = accountutil.LoadAccount(sm, addrv1)
require.NoError(err)
require.Equal("5", s.Balance.String())
require.Equal(uint64(0x0), s.Nonce)
Copy link
Member

Choose a reason for hiding this comment

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

add test to create new type account, assert s.PendingNonce() == 0

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Are you sure to add unit test to this PR?

require.Equal(uint64(0x1), s.PendingNonce())
}

func TestProtocol_Initialize(t *testing.T) {
Expand Down Expand Up @@ -109,7 +109,7 @@ func TestProtocol_Initialize(t *testing.T) {
ctx := protocol.WithBlockCtx(context.Background(), protocol.BlockCtx{
BlockHeight: 0,
})
ctx = genesis.WithGenesisContext(ctx, ge)
ctx = protocol.WithFeatureCtx(genesis.WithGenesisContext(ctx, ge))
p := NewProtocol(rewarding.DepositGas)
require.NoError(
p.CreateGenesisStates(
Expand Down
37 changes: 26 additions & 11 deletions action/protocol/account/transfer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,20 +25,23 @@ const TransferSizeLimit = 32 * 1024

// handleTransfer handles a transfer
func (p *Protocol) handleTransfer(ctx context.Context, act action.Action, sm protocol.StateManager) (*action.Receipt, error) {
actionCtx := protocol.MustGetActionCtx(ctx)
blkCtx := protocol.MustGetBlockCtx(ctx)
tsf, ok := act.(*action.Transfer)
if !ok {
return nil, nil
}
accountCreationOpts := []state.AccountCreationOption{}
if protocol.MustGetFeatureCtx(ctx).CreateLegacyNonceAccount {
accountCreationOpts = append(accountCreationOpts, state.LegacyNonceAccountTypeOption())
}
actionCtx := protocol.MustGetActionCtx(ctx)
// check sender
sender, err := accountutil.LoadOrCreateAccount(sm, actionCtx.Caller)
sender, err := accountutil.LoadOrCreateAccount(sm, actionCtx.Caller, accountCreationOpts...)
if err != nil {
return nil, errors.Wrapf(err, "failed to load or create the account of sender %s", actionCtx.Caller.String())
}

gasFee := big.NewInt(0).Mul(tsf.GasPrice(), big.NewInt(0).SetUint64(actionCtx.IntrinsicGas))
if big.NewInt(0).Add(tsf.Amount(), gasFee).Cmp(sender.Balance) == 1 {
if !sender.HasSufficientBalance(big.NewInt(0).Add(tsf.Amount(), gasFee)) {
Copy link
Member

Choose a reason for hiding this comment

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

I think the original exp is clear enough

return nil, errors.Wrapf(
state.ErrNotEnoughBalance,
"sender %s balance %s, required amount %s",
Expand Down Expand Up @@ -74,13 +77,19 @@ func (p *Protocol) handleTransfer(ctx context.Context, act action.Action, sm pro
if err != nil {
return nil, errors.Wrapf(err, "failed to decode recipient address %s", tsf.Recipient())
}
recipientAcct, err := accountutil.LoadAccount(sm, recipientAddr)
recipientAcct, err := accountutil.LoadAccount(sm, recipientAddr, accountCreationOpts...)
if err != nil {
return nil, errors.Wrapf(err, "failed to load address %s", tsf.Recipient())
}
fixNonce := protocol.MustGetFeatureCtx(ctx).FixGasAndNonceUpdate
blkCtx := protocol.MustGetBlockCtx(ctx)
Comment on lines +84 to +85
Copy link
Member

@Liuhaai Liuhaai Jun 27, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

the names FixGasAndNonceUpdate and fixNonce are confusing. Comments about fix description are needed or get better names for them.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixNonce is initialized with feature FixGasAndNonceUpdate, which fixes both nonce and gas. Here, we only use the fixNonce attribute. Other than that, fixNonce is just a local parameter. I don't get it how could it be confusing.

Copy link
Member

Choose a reason for hiding this comment

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

does it belong to pr Skip update nonce and gas for system actions ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we only have one PR for now.

if recipientAcct.IsContract() {
// update sender Nonce
accountutil.SetNonce(tsf, sender)
if fixNonce || tsf.Nonce() != 0 {
Copy link
Member

Choose a reason for hiding this comment

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

SetPendingNonce can be moved out of if condition?

// update sender Nonce
if err := sender.SetPendingNonce(tsf.Nonce() + 1); err != nil {
return nil, errors.Wrapf(err, "failed to update pending nonce of sender %s", actionCtx.Caller.String())
}
}
// put updated sender's state to trie
if err := accountutil.StoreAccount(sm, actionCtx.Caller, sender); err != nil {
return nil, errors.Wrap(err, "failed to update pending account changes to trie")
Expand Down Expand Up @@ -108,18 +117,24 @@ func (p *Protocol) handleTransfer(ctx context.Context, act action.Action, sm pro
if err := sender.SubBalance(tsf.Amount()); err != nil {
return nil, errors.Wrapf(err, "failed to update the Balance of sender %s", actionCtx.Caller.String())
}
// update sender Nonce
accountutil.SetNonce(tsf, sender)
if fixNonce || tsf.Nonce() != 0 {
// update sender Nonce
if err := sender.SetPendingNonce(tsf.Nonce() + 1); err != nil {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

how could fixNonce be moved into SetPendingNonce?

return nil, errors.Wrapf(err, "failed to update pending nonce of sender %s", actionCtx.Caller.String())
}
}
// put updated sender's state to trie
if err := accountutil.StoreAccount(sm, actionCtx.Caller, sender); err != nil {
return nil, errors.Wrap(err, "failed to update pending account changes to trie")
}
// check recipient
recipient, err := accountutil.LoadOrCreateAccount(sm, recipientAddr)
recipient, err := accountutil.LoadOrCreateAccount(sm, recipientAddr, accountCreationOpts...)
if err != nil {
return nil, errors.Wrapf(err, "failed to load or create the account of recipient %s", tsf.Recipient())
}
recipient.AddBalance(tsf.Amount())
if err := recipient.AddBalance(tsf.Amount()); err != nil {
return nil, errors.Wrapf(err, "failed to add balance %s", tsf.Amount())
}
// put updated recipient's state to trie
if err := accountutil.StoreAccount(sm, recipientAddr, recipient); err != nil {
return nil, errors.Wrap(err, "failed to update pending account changes to trie")
Expand Down
Loading