Skip to content

Commit

Permalink
Merge pull request from GHSA-j494-7x2v-vvvp
Browse files Browse the repository at this point in the history
Fix wrong nonce in relayed failed transaction
  • Loading branch information
iulianpascalau committed May 28, 2023
2 parents 9729547 + 9c9c395 commit babdb14
Show file tree
Hide file tree
Showing 27 changed files with 485 additions and 59 deletions.
3 changes: 3 additions & 0 deletions cmd/node/config/enableEpochs.toml
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,9 @@
# RuntimeCodeSizeFixEnableEpoch represents the epoch when the code size fix in the VM is enabled
RuntimeCodeSizeFixEnableEpoch = 1

# RelayedNonceFixEnableEpoch represents the epoch when the nonce fix for relayed txs is enabled
RelayedNonceFixEnableEpoch = 2

# BLSMultiSignerEnableEpoch represents the activation epoch for different types of BLS multi-signers
BLSMultiSignerEnableEpoch = [
{ EnableEpoch = 0, Type = "no-KOSK"},
Expand Down
6 changes: 6 additions & 0 deletions common/enablers/enableEpochsHandler.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ func (handler *enableEpochsHandler) EpochConfirmed(epoch uint32, _ uint64) {
handler.setFlagValue(epoch >= handler.enableEpochsConfig.MaxBlockchainHookCountersEnableEpoch, handler.maxBlockchainHookCountersFlag, "maxBlockchainHookCountersFlag")
handler.setFlagValue(epoch >= handler.enableEpochsConfig.WipeSingleNFTLiquidityDecreaseEnableEpoch, handler.wipeSingleNFTLiquidityDecreaseFlag, "wipeSingleNFTLiquidityDecreaseFlag")
handler.setFlagValue(epoch >= handler.enableEpochsConfig.AlwaysSaveTokenMetaDataEnableEpoch, handler.alwaysSaveTokenMetaDataFlag, "alwaysSaveTokenMetaDataFlag")
handler.setFlagValue(epoch >= handler.enableEpochsConfig.RelayedNonceFixEnableEpoch, handler.relayedNonceFixFlag, "relayedNonceFixFlag")
}

func (handler *enableEpochsHandler) setFlagValue(value bool, flag *atomic.Flag, flagName string) {
Expand Down Expand Up @@ -215,6 +216,11 @@ func (handler *enableEpochsHandler) RefactorPeersMiniBlocksEnableEpoch() uint32
return handler.enableEpochsConfig.RefactorPeersMiniBlocksEnableEpoch
}

// RelayedNonceFixEnableEpoch returns the epoch when relayed nonce fix becomes active
func (handler *enableEpochsHandler) RelayedNonceFixEnableEpoch() uint32 {
return handler.enableEpochsConfig.RelayedNonceFixEnableEpoch
}

// IsInterfaceNil returns true if there is no value under the interface
func (handler *enableEpochsHandler) IsInterfaceNil() bool {
return handler == nil
Expand Down
9 changes: 7 additions & 2 deletions common/enablers/enableEpochsHandler_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package enablers

import (
"math"
"testing"

"github.com/multiversx/mx-chain-core-go/core/check"
Expand Down Expand Up @@ -91,6 +92,7 @@ func createEnableEpochsConfig() config.EnableEpochs {
WipeSingleNFTLiquidityDecreaseEnableEpoch: 75,
AlwaysSaveTokenMetaDataEnableEpoch: 76,
RuntimeCodeSizeFixEnableEpoch: 77,
RelayedNonceFixEnableEpoch: 78,
}
}

Expand Down Expand Up @@ -129,7 +131,7 @@ func TestNewEnableEpochsHandler_EpochConfirmed(t *testing.T) {
handler, _ := NewEnableEpochsHandler(cfg, &epochNotifier.EpochNotifierStub{})
require.False(t, check.IfNil(handler))

handler.EpochConfirmed(77, 0)
handler.EpochConfirmed(math.MaxUint32, 0)

assert.Equal(t, cfg.BlockGasAndFeesReCheckEnableEpoch, handler.BlockGasAndFeesReCheckEnableEpoch())
assert.True(t, handler.IsSCDeployFlagEnabled())
Expand Down Expand Up @@ -213,11 +215,12 @@ func TestNewEnableEpochsHandler_EpochConfirmed(t *testing.T) {
assert.True(t, handler.IsMaxBlockchainHookCountersFlagEnabled())
assert.True(t, handler.IsAlwaysSaveTokenMetaDataEnabled())
assert.True(t, handler.IsRuntimeCodeSizeFixEnabled())
assert.True(t, handler.IsRelayedNonceFixEnabled())
})
t.Run("flags with == condition should be set, along with all >=", func(t *testing.T) {
t.Parallel()

epoch := uint32(78)
epoch := uint32(math.MaxUint32)
cfg := createEnableEpochsConfig()
cfg.StakingV2EnableEpoch = epoch
cfg.ESDTEnableEpoch = epoch
Expand Down Expand Up @@ -313,6 +316,7 @@ func TestNewEnableEpochsHandler_EpochConfirmed(t *testing.T) {
assert.True(t, handler.IsWipeSingleNFTLiquidityDecreaseEnabled())
assert.True(t, handler.IsAlwaysSaveTokenMetaDataEnabled())
assert.True(t, handler.IsRuntimeCodeSizeFixEnabled())
assert.True(t, handler.IsRelayedNonceFixEnabled())
})
t.Run("flags with < should be set", func(t *testing.T) {
t.Parallel()
Expand Down Expand Up @@ -408,5 +412,6 @@ func TestNewEnableEpochsHandler_EpochConfirmed(t *testing.T) {
assert.False(t, handler.IsWipeSingleNFTLiquidityDecreaseEnabled())
assert.False(t, handler.IsAlwaysSaveTokenMetaDataEnabled())
assert.False(t, handler.IsRuntimeCodeSizeFixEnabled())
assert.False(t, handler.IsRelayedNonceFixEnabled())
})
}
7 changes: 7 additions & 0 deletions common/enablers/epochFlags.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ type epochFlagsHolder struct {
maxBlockchainHookCountersFlag *atomic.Flag
wipeSingleNFTLiquidityDecreaseFlag *atomic.Flag
alwaysSaveTokenMetaDataFlag *atomic.Flag
relayedNonceFixFlag *atomic.Flag
}

func newEpochFlagsHolder() *epochFlagsHolder {
Expand Down Expand Up @@ -179,6 +180,7 @@ func newEpochFlagsHolder() *epochFlagsHolder {
maxBlockchainHookCountersFlag: &atomic.Flag{},
wipeSingleNFTLiquidityDecreaseFlag: &atomic.Flag{},
alwaysSaveTokenMetaDataFlag: &atomic.Flag{},
relayedNonceFixFlag: &atomic.Flag{},
}
}

Expand Down Expand Up @@ -659,3 +661,8 @@ func (holder *epochFlagsHolder) IsWipeSingleNFTLiquidityDecreaseEnabled() bool {
func (holder *epochFlagsHolder) IsAlwaysSaveTokenMetaDataEnabled() bool {
return holder.alwaysSaveTokenMetaDataFlag.IsSet()
}

// IsRelayedNonceFixEnabled returns true if relayedNonceFixFlag is enabled
func (holder *epochFlagsHolder) IsRelayedNonceFixEnabled() bool {
return holder.relayedNonceFixFlag.IsSet()
}
1 change: 1 addition & 0 deletions common/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,7 @@ type EnableEpochsHandler interface {
IsMaxBlockchainHookCountersFlagEnabled() bool
IsWipeSingleNFTLiquidityDecreaseEnabled() bool
IsAlwaysSaveTokenMetaDataEnabled() bool
IsRelayedNonceFixEnabled() bool

IsInterfaceNil() bool
}
1 change: 1 addition & 0 deletions config/epochConfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ type EnableEpochs struct {
MaxBlockchainHookCountersEnableEpoch uint32
WipeSingleNFTLiquidityDecreaseEnableEpoch uint32
AlwaysSaveTokenMetaDataEnableEpoch uint32
RelayedNonceFixEnableEpoch uint32
BLSMultiSignerEnableEpoch []MultiSignerConfig
}

Expand Down
4 changes: 4 additions & 0 deletions config/tomlConfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -684,6 +684,9 @@ func TestEnableEpochConfig(t *testing.T) {
# RuntimeMemStoreLimitEnableEpoch represents the epoch when the condition for Runtime MemStore is enabled
RuntimeMemStoreLimitEnableEpoch = 63
# RelayedNonceFixEnableEpoch represents the epoch when the nonce fix for relayed txs is enabled
RelayedNonceFixEnableEpoch = 64
# MaxNodesChangeEnableEpoch holds configuration for changing the maximum number of nodes and the enabling epoch
MaxNodesChangeEnableEpoch = [
{ EpochEnable = 44, MaxNumNodes = 2169, NodesToShufflePerShard = 80 },
Expand Down Expand Up @@ -779,6 +782,7 @@ func TestEnableEpochConfig(t *testing.T) {
AlwaysSaveTokenMetaDataEnableEpoch: 61,
RuntimeCodeSizeFixEnableEpoch: 62,
RuntimeMemStoreLimitEnableEpoch: 63,
RelayedNonceFixEnableEpoch: 64,
BLSMultiSignerEnableEpoch: []MultiSignerConfig{
{
EnableEpoch: 0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
"github.com/stretchr/testify/assert"
)

func TestRelayedTransactionInMultiShardEnvironmentWithNormalTxButWrongNonce(t *testing.T) {
func TestRelayedTransactionInMultiShardEnvironmentWithNormalTxButWrongNonceShouldNotIncrementUserAccNonce(t *testing.T) {
if testing.Short() {
t.Skip("this is not a short test")
}
Expand Down Expand Up @@ -68,7 +68,7 @@ func TestRelayedTransactionInMultiShardEnvironmentWithNormalTxButWrongNonce(t *t
for _, player := range players {
account := relayedTx.GetUserAccount(nodes, player.Address)
assert.True(t, account.GetBalance().Cmp(big.NewInt(0)) == 0)
assert.Equal(t, uint64(nrRoundsToTest)*2, account.GetNonce())
assert.Equal(t, uint64(0), account.GetNonce())
}

expectedBalance := big.NewInt(0).Sub(relayerInitialValue, totalFees)
Expand Down
20 changes: 20 additions & 0 deletions integrationTests/testInitializer.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ import (
testStorage "github.com/multiversx/mx-chain-go/testscommon/state"
"github.com/multiversx/mx-chain-go/testscommon/statusHandler"
statusHandlerMock "github.com/multiversx/mx-chain-go/testscommon/statusHandler"
"github.com/multiversx/mx-chain-go/testscommon/txDataBuilder"
"github.com/multiversx/mx-chain-go/trie"
"github.com/multiversx/mx-chain-go/trie/hashesHolder"
"github.com/multiversx/mx-chain-go/vm"
Expand Down Expand Up @@ -2538,3 +2539,22 @@ func SaveDelegationContractsList(nodes []*TestProcessorNode) {
_, _ = n.AccntState.Commit()
}
}

// PrepareRelayedTxDataV1 repares the data for a relayed transaction V1
func PrepareRelayedTxDataV1(innerTx *transaction.Transaction) []byte {
userTxBytes, _ := TestMarshalizer.Marshal(innerTx)
return []byte(core.RelayedTransaction + "@" + hex.EncodeToString(userTxBytes))
}

// PrepareRelayedTxDataV2 prepares the data for a relayed transaction V2
func PrepareRelayedTxDataV2(innerTx *transaction.Transaction) []byte {
dataBuilder := txDataBuilder.NewBuilder()
txData := dataBuilder.
Func(core.RelayedTransactionV2).
Bytes(innerTx.RcvAddr).
Int64(int64(innerTx.Nonce)).
Bytes(innerTx.Data).
Bytes(innerTx.Signature)

return txData.ToBytes()
}
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func TestRelayedBuiltInFunctionExecuteOnRelayerAndDstShardShouldWork(t *testing.

_, _ = vm.CreateAccount(testContextRelayer.Accounts, relayerAddr, 0, big.NewInt(15000))

rtxData := utils.PrepareRelayerTxData(innerTx)
rtxData := integrationTests.PrepareRelayedTxDataV1(innerTx)
rTxGasLimit := 1 + gasLimit + uint64(len(rtxData))
rtx := vm.CreateTransaction(0, innerTx.Value, relayerAddr, owner, gasPrice, rTxGasLimit, rtxData)

Expand Down
13 changes: 7 additions & 6 deletions integrationTests/vm/txsFee/multiShard/relayedMoveBalance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"testing"

"github.com/multiversx/mx-chain-go/config"
"github.com/multiversx/mx-chain-go/integrationTests"
"github.com/multiversx/mx-chain-go/integrationTests/vm"
"github.com/multiversx/mx-chain-go/integrationTests/vm/txsFee/utils"
vmcommon "github.com/multiversx/mx-chain-vm-common-go"
Expand Down Expand Up @@ -34,7 +35,7 @@ func TestRelayedMoveBalanceRelayerShard0InnerTxSenderAndReceiverShard1ShouldWork

userTx := vm.CreateTransaction(0, big.NewInt(100), sndAddr, rcvAddr, gasPrice, gasLimit, []byte("aaaa"))

rtxData := utils.PrepareRelayerTxData(userTx)
rtxData := integrationTests.PrepareRelayedTxDataV1(userTx)
rTxGasLimit := 1 + gasLimit + uint64(len(rtxData))
rtx := vm.CreateTransaction(0, userTx.Value, relayerAddr, sndAddr, gasPrice, rTxGasLimit, rtxData)

Expand Down Expand Up @@ -80,7 +81,7 @@ func TestRelayedMoveBalanceRelayerAndInnerTxSenderShard0ReceiverShard1(t *testin

userTx := vm.CreateTransaction(0, big.NewInt(100), sndAddr, scAddrBytes, gasPrice, gasLimit, nil)

rtxData := utils.PrepareRelayerTxData(userTx)
rtxData := integrationTests.PrepareRelayedTxDataV1(userTx)
rTxGasLimit := 1 + gasLimit + uint64(len(rtxData))
rtx := vm.CreateTransaction(0, userTx.Value, relayerAddr, sndAddr, gasPrice, rTxGasLimit, rtxData)

Expand Down Expand Up @@ -131,7 +132,7 @@ func TestRelayedMoveBalanceExecuteOnSourceAndDestination(t *testing.T) {

userTx := vm.CreateTransaction(0, big.NewInt(100), sndAddr, scAddrBytes, gasPrice, gasLimit, nil)

rtxData := utils.PrepareRelayerTxData(userTx)
rtxData := integrationTests.PrepareRelayedTxDataV1(userTx)
rTxGasLimit := 1 + gasLimit + uint64(len(rtxData))
rtx := vm.CreateTransaction(0, userTx.Value, relayerAddr, sndAddr, gasPrice, rTxGasLimit, rtxData)

Expand Down Expand Up @@ -193,7 +194,7 @@ func TestRelayedMoveBalanceExecuteOnSourceAndDestinationRelayerAndInnerTxSenderS

userTx := vm.CreateTransaction(0, big.NewInt(100), sndAddr, rcvAddr, gasPrice, gasLimit, nil)

rtxData := utils.PrepareRelayerTxData(userTx)
rtxData := integrationTests.PrepareRelayedTxDataV1(userTx)
rTxGasLimit := 1 + gasLimit + uint64(len(rtxData))
rtx := vm.CreateTransaction(0, userTx.Value, relayerAddr, sndAddr, gasPrice, rTxGasLimit, rtxData)

Expand Down Expand Up @@ -253,7 +254,7 @@ func TestRelayedMoveBalanceRelayerAndInnerTxReceiverShard0SenderShard1(t *testin

innerTx := vm.CreateTransaction(0, big.NewInt(100), sndAddr, rcvAddr, gasPrice, gasLimit, nil)

rtxData := utils.PrepareRelayerTxData(innerTx)
rtxData := integrationTests.PrepareRelayedTxDataV1(innerTx)
rTxGasLimit := 1 + gasLimit + uint64(len(rtxData))
rtx := vm.CreateTransaction(0, innerTx.Value, relayerAddr, sndAddr, gasPrice, rTxGasLimit, rtxData)

Expand Down Expand Up @@ -329,7 +330,7 @@ func TestMoveBalanceRelayerShard0InnerTxSenderShard1InnerTxReceiverShard2ShouldW

innerTx := vm.CreateTransaction(0, big.NewInt(100), sndAddr, rcvAddr, gasPrice, gasLimit, nil)

rtxData := utils.PrepareRelayerTxData(innerTx)
rtxData := integrationTests.PrepareRelayedTxDataV1(innerTx)
rTxGasLimit := 1 + gasLimit + uint64(len(rtxData))
rtx := vm.CreateTransaction(0, innerTx.Value, relayerAddr, sndAddr, gasPrice, rTxGasLimit, rtxData)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"testing"

"github.com/multiversx/mx-chain-go/config"
"github.com/multiversx/mx-chain-go/integrationTests"
"github.com/multiversx/mx-chain-go/integrationTests/vm"
"github.com/multiversx/mx-chain-go/integrationTests/vm/txsFee/utils"
"github.com/multiversx/mx-chain-go/integrationTests/vm/wasm"
Expand Down Expand Up @@ -39,7 +40,7 @@ func TestRelayedSCDeployShouldWork(t *testing.T) {
scCode := wasm.GetSCCode(contractPath)
userTx := vm.CreateTransaction(0, big.NewInt(0), sndAddr, vm.CreateEmptyAddress(), gasPrice, gasLimit, []byte(wasm.CreateDeployTxData(scCode)))

rtxData := utils.PrepareRelayerTxData(userTx)
rtxData := integrationTests.PrepareRelayedTxDataV1(userTx)
rTxGasLimit := 1 + gasLimit + uint64(len(rtxData))
rtx := vm.CreateTransaction(0, big.NewInt(0), relayerAddr, sndAddr, gasPrice, rTxGasLimit, rtxData)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"testing"

"github.com/multiversx/mx-chain-go/config"
"github.com/multiversx/mx-chain-go/integrationTests"
"github.com/multiversx/mx-chain-go/integrationTests/vm"
"github.com/multiversx/mx-chain-go/integrationTests/vm/txsFee/utils"
vmcommon "github.com/multiversx/mx-chain-vm-common-go"
Expand Down Expand Up @@ -57,7 +58,7 @@ func TestRelayedTxScCallMultiShardShouldWork(t *testing.T) {
gasLimit := uint64(500)

innerTx := vm.CreateTransaction(0, big.NewInt(0), sndAddr, scAddr, gasPrice, gasLimit, []byte("increment"))
rtxData := utils.PrepareRelayerTxData(innerTx)
rtxData := integrationTests.PrepareRelayedTxDataV1(innerTx)
rTxGasLimit := 1 + gasLimit + uint64(len(rtxData))
rtx := vm.CreateTransaction(0, innerTx.Value, relayerAddr, sndAddr, gasPrice, rTxGasLimit, rtxData)

Expand Down Expand Up @@ -162,7 +163,7 @@ func TestRelayedTxScCallMultiShardFailOnInnerTxDst(t *testing.T) {
gasLimit := uint64(500)

innerTx := vm.CreateTransaction(0, big.NewInt(0), sndAddr, scAddr, gasPrice, gasLimit, []byte("incremeno"))
rtxData := utils.PrepareRelayerTxData(innerTx)
rtxData := integrationTests.PrepareRelayedTxDataV1(innerTx)
rTxGasLimit := 1 + gasLimit + uint64(len(rtxData))
rtx := vm.CreateTransaction(0, innerTx.Value, relayerAddr, sndAddr, gasPrice, rTxGasLimit, rtxData)

Expand Down
33 changes: 29 additions & 4 deletions integrationTests/vm/txsFee/relayedAsyncCall_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,42 @@ import (
"testing"

"github.com/multiversx/mx-chain-go/config"
"github.com/multiversx/mx-chain-go/integrationTests"
"github.com/multiversx/mx-chain-go/integrationTests/vm"
"github.com/multiversx/mx-chain-go/integrationTests/vm/txsFee/utils"
vmcommon "github.com/multiversx/mx-chain-vm-common-go"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestRelayedAsyncCallShouldWork(t *testing.T) {
testContext, err := vm.CreatePreparedTxProcessorWithVMs(config.EnableEpochs{})
senderAddr := []byte("12345678901234567890123456789011")

t.Run("nonce fix is disabled, should increase the sender's nonce", func(t *testing.T) {
testContext := testRelayedAsyncCallShouldWork(t, config.EnableEpochs{
RelayedNonceFixEnableEpoch: 100000,
}, senderAddr)
defer testContext.Close()

senderAccount := getAccount(t, testContext, senderAddr)
assert.Equal(t, uint64(1), senderAccount.GetNonce())
})
t.Run("nonce fix is enabled, should still increase the sender's nonce", func(t *testing.T) {
testContext := testRelayedAsyncCallShouldWork(t, config.EnableEpochs{
RelayedNonceFixEnableEpoch: 0,
}, senderAddr)
defer testContext.Close()

senderAccount := getAccount(t, testContext, senderAddr)
assert.Equal(t, uint64(1), senderAccount.GetNonce())
})
}

func testRelayedAsyncCallShouldWork(t *testing.T, enableEpochs config.EnableEpochs, senderAddr []byte) *vm.VMTestContext {
testContext, err := vm.CreatePreparedTxProcessorWithVMs(enableEpochs)
require.Nil(t, err)
defer testContext.Close()

egldBalance := big.NewInt(100000000)
senderAddr := []byte("12345678901234567890123456789011")
ownerAddr := []byte("12345678901234567890123456789010")
relayerAddr := []byte("12345678901234567890123456789017")

Expand All @@ -48,7 +71,7 @@ func TestRelayedAsyncCallShouldWork(t *testing.T) {

innerTx := vm.CreateTransaction(0, big.NewInt(0), senderAddr, secondSCAddress, gasPrice, gasLimit, []byte("doSomething"))

rtxData := utils.PrepareRelayerTxData(innerTx)
rtxData := integrationTests.PrepareRelayedTxDataV1(innerTx)
rTxGasLimit := 1 + gasLimit + uint64(len(rtxData))
rtx := vm.CreateTransaction(0, innerTx.Value, relayerAddr, senderAddr, gasPrice, rTxGasLimit, rtxData)

Expand All @@ -69,4 +92,6 @@ func TestRelayedAsyncCallShouldWork(t *testing.T) {

require.Equal(t, big.NewInt(50001950), testContext.TxFeeHandler.GetAccumulatedFees())
require.Equal(t, big.NewInt(4999988), testContext.TxFeeHandler.GetDeveloperFees())

return testContext
}
Loading

0 comments on commit babdb14

Please sign in to comment.