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

Change send action #1544

Merged
merged 9 commits into from
Oct 15, 2019
Merged

Change send action #1544

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c4f8488
change sendaction to add to local pool directly
koseoyoung Oct 8, 2019
c62ae33
refactor
koseoyoung Oct 10, 2019
bc06420
fix unit test
koseoyoung Oct 10, 2019
7cd19ae
fix unit test
koseoyoung Oct 11, 2019
a8c18f3
Merge branch 'master' into change_send_action
koseoyoung Oct 11, 2019
48f83f9
fixed unit tests
koseoyoung Oct 12, 2019
59f303c
wrap error with status
koseoyoung Oct 14, 2019
313d1d2
Merge branch 'master' into change_send_action
koseoyoung Oct 14, 2019
f4698eb
fix unit test
koseoyoung Oct 15, 2019
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
30 changes: 14 additions & 16 deletions api/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ import (
"github.com/iotexproject/iotex-core/blockchain/block"
"github.com/iotexproject/iotex-core/config"
"github.com/iotexproject/iotex-core/db"
"github.com/iotexproject/iotex-core/dispatcher"
"github.com/iotexproject/iotex-core/gasstation"
"github.com/iotexproject/iotex-core/pkg/log"
"github.com/iotexproject/iotex-core/pkg/util/byteutil"
Expand Down Expand Up @@ -88,7 +87,6 @@ func WithNativeElection(committee committee.Committee) Option {
// Server provides api for user to query blockchain data
type Server struct {
bc blockchain.Blockchain
dp dispatcher.Dispatcher
ap actpool.ActPool
gs *gasstation.GasStation
broadcastHandler BroadcastOutbound
Expand All @@ -104,7 +102,6 @@ type Server struct {
func NewServer(
cfg config.Config,
chain blockchain.Blockchain,
dispatcher dispatcher.Dispatcher,
actPool actpool.ActPool,
registry *protocol.Registry,
opts ...Option,
Expand All @@ -127,7 +124,6 @@ func NewServer(

svr := &Server{
bc: chain,
dp: dispatcher,
ap: actPool,
broadcastHandler: apiCfg.broadcastHandler,
cfg: cfg,
Expand Down Expand Up @@ -302,22 +298,24 @@ func (api *Server) GetServerMeta(ctx context.Context,
}

// SendAction is the API to send an action to blockchain.
func (api *Server) SendAction(ctx context.Context, in *iotexapi.SendActionRequest) (res *iotexapi.SendActionResponse, err error) {
func (api *Server) SendAction(ctx context.Context, in *iotexapi.SendActionRequest) (*iotexapi.SendActionResponse, error) {
Copy link

Choose a reason for hiding this comment

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

line is 122 characters (from lll)

log.L().Debug("receive send action request")

// broadcast to the network
if err = api.broadcastHandler(context.Background(), api.bc.ChainID(), in.Action); err != nil {
log.L().Warn("Failed to broadcast SendAction request.", zap.Error(err))
}
// send to actpool via dispatcher
api.dp.HandleBroadcast(context.Background(), api.bc.ChainID(), in.Action)

var selp action.SealedEnvelope
var err error
Copy link

Choose a reason for hiding this comment

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

declarations should never be cuddled (from wsl)

if err = selp.LoadProto(in.Action); err != nil {
return
return nil, status.Error(codes.InvalidArgument, err.Error())
}
// Add to local actpool
if err = api.ap.Add(selp); err != nil {
log.L().Debug(err.Error())
return nil, status.Error(codes.Internal, err.Error())
}
// If there is no error putting into local actpool,
// Broadcast it to the network
if err = api.broadcastHandler(context.Background(), api.bc.ChainID(), in.Action); err != nil {
log.L().Warn("Failed to broadcast SendAction request.", zap.Error(err))
}
hash := selp.Hash()

return &iotexapi.SendActionResponse{ActionHash: hex.EncodeToString(hash[:])}, nil
}

Expand Down Expand Up @@ -411,7 +409,7 @@ func (api *Server) EstimateGasForAction(ctx context.Context, in *iotexapi.Estima
return &iotexapi.EstimateGasForActionResponse{Gas: estimateGas}, nil
}

// EstimateActionGasConsumption estimate gas consume for exectution and transfer
// EstimateActionGasConsumption estimate gas consume for execution and transfer
func (api *Server) EstimateActionGasConsumption(ctx context.Context, in *iotexapi.EstimateActionGasConsumptionRequest) (*iotexapi.EstimateActionGasConsumptionResponse, error) {
switch {
case in.GetExecution() != nil:
Expand Down
10 changes: 5 additions & 5 deletions api/api_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ import (
"github.com/iotexproject/iotex-core/state"
"github.com/iotexproject/iotex-core/state/factory"
"github.com/iotexproject/iotex-core/test/identityset"
"github.com/iotexproject/iotex-core/test/mock/mock_actpool"
"github.com/iotexproject/iotex-core/test/mock/mock_blockchain"
"github.com/iotexproject/iotex-core/test/mock/mock_dispatcher"
"github.com/iotexproject/iotex-core/test/mock/mock_factory"
"github.com/iotexproject/iotex-core/testutil"
"github.com/iotexproject/iotex-election/test/mock/mock_committee"
Expand Down Expand Up @@ -953,15 +953,15 @@ func TestServer_SendAction(t *testing.T) {
defer ctrl.Finish()

chain := mock_blockchain.NewMockBlockchain(ctrl)
mDp := mock_dispatcher.NewMockDispatcher(ctrl)
ap := mock_actpool.NewMockActPool(ctrl)
broadcastHandlerCount := 0
svr := Server{bc: chain, dp: mDp, broadcastHandler: func(_ context.Context, _ uint32, _ proto.Message) error {
svr := Server{bc: chain, ap: ap, broadcastHandler: func(_ context.Context, _ uint32, _ proto.Message) error {
broadcastHandlerCount++
return nil
}}

chain.EXPECT().ChainID().Return(uint32(1)).Times(4)
mDp.EXPECT().HandleBroadcast(gomock.Any(), gomock.Any(), gomock.Any()).Times(2)
chain.EXPECT().ChainID().Return(uint32(1)).Times(2)
ap.EXPECT().Add(gomock.Any()).Return(nil).Times(2)

for i, test := range sendActionTests {
request := &iotexapi.SendActionRequest{Action: test.actionPb}
Expand Down
1 change: 0 additions & 1 deletion chainservice/chainservice.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,6 @@ func New(
apiSvr, err = api.NewServer(
cfg,
chain,
dispatcher,
actPool,
&registry,
api.WithBroadcastOutbound(func(ctx context.Context, chainID uint32, msg proto.Message) error {
Expand Down
6 changes: 4 additions & 2 deletions e2etest/local_transfer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,10 +315,9 @@ func TestLocalTransfer(t *testing.T) {
_, err := client.SendAction(context.Background(), &iotexapi.SendActionRequest{Action: tsf.Proto()})
return err
}, bo)
require.NoError(err, tsfTest.message)

switch tsfTest.expectedResult {
case TsfSuccess:
require.NoError(err, tsfTest.message)
//Wait long enough for a block to be minted, and check the balance of both
//sender and receiver.
var selp action.SealedEnvelope
Expand Down Expand Up @@ -351,6 +350,7 @@ func TestLocalTransfer(t *testing.T) {
}
require.Equal(expectedRecvrBalance.String(), newRecvBalance.String(), tsfTest.message)
case TsfFail:
require.Error(err, tsfTest.message)
//The transfer should be rejected right after we inject it
//Wait long enough to make sure the failed transfer does not exit in either action pool or blockchain
err := backoff.Retry(func() error {
Expand All @@ -368,6 +368,7 @@ func TestLocalTransfer(t *testing.T) {
}

case TsfPending:
require.NoError(err, tsfTest.message)
//Need to wait long enough to make sure the pending transfer is not minted, only stay in action pool
err := backoff.Retry(func() error {
var err error
Expand All @@ -378,6 +379,7 @@ func TestLocalTransfer(t *testing.T) {
_, err = bc.GetActionByActionHash(tsf.Hash())
require.Error(err, tsfTest.message)
case TsfFinal:
require.NoError(err, tsfTest.message)
//After a blocked is minted, check all the pending transfers in action pool are cleared
//This checking procedure is simplified for this test case, because of the complexity of
//handling pending transfers.
Expand Down
6 changes: 2 additions & 4 deletions tools/actioninjector.v2/internal/client/client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (
"github.com/iotexproject/iotex-core/test/identityset"
"github.com/iotexproject/iotex-core/test/mock/mock_actpool"
"github.com/iotexproject/iotex-core/test/mock/mock_blockchain"
"github.com/iotexproject/iotex-core/test/mock/mock_dispatcher"
"github.com/iotexproject/iotex-core/testutil"
)

Expand Down Expand Up @@ -45,18 +44,17 @@ func TestClient(t *testing.T) {

bc := mock_blockchain.NewMockBlockchain(mockCtrl)
ap := mock_actpool.NewMockActPool(mockCtrl)
dp := mock_dispatcher.NewMockDispatcher(mockCtrl)

bc.EXPECT().StateByAddr(gomock.Any()).Return(&state, nil).AnyTimes()
bc.EXPECT().ChainID().Return(chainID).AnyTimes()
bc.EXPECT().AddSubscriber(gomock.Any()).Return(nil).AnyTimes()
bc.EXPECT().GetActionCountByAddress(gomock.Any()).Return(uint64(1), nil).AnyTimes()
ap.EXPECT().GetPendingNonce(gomock.Any()).Return(uint64(1), nil).AnyTimes()
dp.EXPECT().HandleBroadcast(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes()
ap.EXPECT().Add(gomock.Any()).Return(nil).AnyTimes()
newOption := api.WithBroadcastOutbound(func(_ context.Context, _ uint32, _ proto.Message) error {
return nil
})
apiServer, err := api.NewServer(cfg, bc, dp, ap, nil, newOption)
apiServer, err := api.NewServer(cfg, bc, ap, nil, newOption)
require.NoError(err)
require.NoError(apiServer.Start())
// test New()
Expand Down