Skip to content

Commit

Permalink
node: governor custom reset release timer delay (#3983)
Browse files Browse the repository at this point in the history
* Update ChainGovernorResetReleaseTimerRequest protobuf message

* Add numDays argument to governor-reset-release-timer command

* Update governor backend to support numDays argument

* Address review comments

* Add test for resetReleaseTimerForTime()'s numDays parameter

* Address review comments

* Add adminrpc test for ChainGovernorResetReleaseTimer

* Replace hardcoded upper boundaries with maxResetReleaseTimerDays

* Update governor whitepaper to reflect the new argument

* Added default value to governor whitepaper

---------

Co-authored-by: Jason Matthyser <jason@asymmetric.re>
  • Loading branch information
pleasew8t and Jason Matthyser authored Jun 20, 2024
1 parent 305cb8f commit 627faa7
Show file tree
Hide file tree
Showing 8 changed files with 363 additions and 187 deletions.
21 changes: 17 additions & 4 deletions node/cmd/guardiand/adminclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,10 +164,10 @@ var ClientChainGovernorReleasePendingVAACmd = &cobra.Command{
}

var ClientChainGovernorResetReleaseTimerCmd = &cobra.Command{
Use: "governor-reset-release-timer [VAA_ID]",
Short: "Resets the release timer for a chain governor pending VAA, extending it to the configured maximum",
Use: "governor-reset-release-timer [VAA_ID] <num_days>",
Short: "Resets the release timer for a chain governor pending VAA, extending it to num_days (up to a maximum of 7), defaulting to one day if num_days is omitted",
Run: runChainGovernorResetReleaseTimer,
Args: cobra.ExactArgs(1),
Args: cobra.RangeArgs(1, 2),
}

var PurgePythNetVaasCmd = &cobra.Command{
Expand Down Expand Up @@ -549,8 +549,21 @@ func runChainGovernorResetReleaseTimer(cmd *cobra.Command, args []string) {
}
defer conn.Close()

// defaults to 1 day if num_days isn't specified
numDays := uint32(1)
if len(args) > 1 {
numDaysArg, err := strconv.Atoi(args[1])

if err != nil {
log.Fatalf("invalid num_days: %v", err)
}

numDays = uint32(numDaysArg)
}

msg := nodev1.ChainGovernorResetReleaseTimerRequest{
VaaId: args[0],
VaaId: args[0],
NumDays: numDays,
}
resp, err := c.ChainGovernorResetReleaseTimer(ctx, &msg)
if err != nil {
Expand Down
8 changes: 7 additions & 1 deletion node/pkg/adminrpc/adminserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ import (
sdktypes "github.com/cosmos/cosmos-sdk/types"
)

const maxResetReleaseTimerDays = 7

var (
vaaInjectionsTotal = promauto.NewCounter(
prometheus.CounterOpts{
Expand Down Expand Up @@ -1031,7 +1033,11 @@ func (s *nodePrivilegedService) ChainGovernorResetReleaseTimer(ctx context.Conte
return nil, fmt.Errorf("the VAA id must be specified as \"chainId/emitterAddress/seqNum\"")
}

resp, err := s.governor.ResetReleaseTimer(req.VaaId)
if req.NumDays < 1 || req.NumDays > maxResetReleaseTimerDays {
return nil, fmt.Errorf("the specified number of days falls outside the range of 1 to %d", maxResetReleaseTimerDays)
}

resp, err := s.governor.ResetReleaseTimer(req.VaaId, req.NumDays)
if err != nil {
return nil, err
}
Expand Down
84 changes: 84 additions & 0 deletions node/pkg/adminrpc/adminserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ import (
"testing"
"time"

wh_common "github.com/certusone/wormhole/node/pkg/common"
"github.com/certusone/wormhole/node/pkg/db"
"github.com/certusone/wormhole/node/pkg/governor"
nodev1 "github.com/certusone/wormhole/node/pkg/proto/node/v1"
"github.com/certusone/wormhole/node/pkg/watchers/evm/connectors"
"github.com/certusone/wormhole/node/pkg/watchers/evm/connectors/ethabi"
Expand Down Expand Up @@ -317,3 +320,84 @@ func Test_adminCommands(t *testing.T) {
})
}
}

func newNodePrivilegedServiceForGovernorTests() *nodePrivilegedService {
gov := governor.NewChainGovernor(zap.NewNop(), &db.MockGovernorDB{}, wh_common.GoTest)

return &nodePrivilegedService{
db: nil,
injectC: nil,
obsvReqSendC: nil,
logger: nil,
signedInC: nil,
governor: gov,
evmConnector: nil,
gk: nil,
guardianAddress: common.Address{},
}
}

func TestChainGovernorResetReleaseTimer(t *testing.T) {
service := newNodePrivilegedServiceForGovernorTests()

// governor has no VAAs enqueued, so if we receive this error we know the input validation passed
success := `vaa not found in the pending list`
boundsCheckFailure := `the specified number of days falls outside the range of 1 to 7`
vaaIdLengthFailure := `the VAA id must be specified as "chainId/emitterAddress/seqNum"`

tests := map[string]struct {
vaaId string
numDays uint32
expectedResult string
}{
"EmptyVaaId": {
vaaId: "",
numDays: 1,
expectedResult: vaaIdLengthFailure,
},
"NumDaysEqualsLowerBoundary": {
vaaId: "valid",
numDays: 1,
expectedResult: success,
},
"NumDaysLowerThanLowerBoundary": {
vaaId: "valid",
numDays: 0,
expectedResult: boundsCheckFailure,
},
"NumDaysEqualsUpperBoundary": {
vaaId: "valid",
numDays: maxResetReleaseTimerDays,
expectedResult: success,
},
"NumDaysExceedsUpperBoundary": {
vaaId: "valid",
numDays: maxResetReleaseTimerDays + 1,
expectedResult: boundsCheckFailure,
},
"EmptyVaaIdAndNumDaysExceedsUpperBoundary": {
vaaId: "",
numDays: maxResetReleaseTimerDays + 1,
expectedResult: vaaIdLengthFailure,
},
"NumDaysSignificantlyExceedsUpperBoundary": {
vaaId: "valid",
numDays: maxResetReleaseTimerDays + 1000,
expectedResult: boundsCheckFailure,
},
}

ctx := context.Background()
for name, test := range tests {
t.Run(name, func(t *testing.T) {
req := nodev1.ChainGovernorResetReleaseTimerRequest{
VaaId: test.vaaId,
NumDays: test.numDays,
}

_, err := service.ChainGovernorResetReleaseTimer(ctx, &req)
assert.EqualError(t, err, test.expectedResult)
})
}

}
8 changes: 4 additions & 4 deletions node/pkg/governor/governor_monitoring.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,19 +213,19 @@ func (gov *ChainGovernor) ReleasePendingVAA(vaaId string) (string, error) {
}

// Admin command to reset the release timer for a pending VAA, extending it to the configured limit.
func (gov *ChainGovernor) ResetReleaseTimer(vaaId string) (string, error) {
return gov.resetReleaseTimerForTime(vaaId, time.Now())
func (gov *ChainGovernor) ResetReleaseTimer(vaaId string, numDays uint32) (string, error) {
return gov.resetReleaseTimerForTime(vaaId, time.Now(), numDays)
}

func (gov *ChainGovernor) resetReleaseTimerForTime(vaaId string, now time.Time) (string, error) {
func (gov *ChainGovernor) resetReleaseTimerForTime(vaaId string, now time.Time, numDays uint32) (string, error) {
gov.mutex.Lock()
defer gov.mutex.Unlock()

for _, ce := range gov.chains {
for _, pe := range ce.pending {
msgId := pe.dbData.Msg.MessageIDString()
if msgId == vaaId {
pe.dbData.ReleaseTime = now.Add(maxEnqueuedTime)
pe.dbData.ReleaseTime = now.Add(time.Duration(numDays) * time.Hour * 24)
gov.logger.Info("updating the release time due to admin command",
zap.String("msgId", msgId),
zap.Stringer("timeStamp", pe.dbData.Msg.Timestamp),
Expand Down
65 changes: 64 additions & 1 deletion node/pkg/governor/governor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1310,6 +1310,69 @@ func TestTestnetConfigIsValid(t *testing.T) {
require.NoError(t, err)
}

func TestNumDaysForReleaseTimerReset(t *testing.T) {
ctx := context.Background()
gov, err := newChainGovernorForTest(ctx)

require.NoError(t, err)

tokenAddrStr := "0xDDb64fE46a91D46ee29420539FC25FD07c5FEa3E" //nolint:gosec
toAddrStr := "0x707f9118e33a9b8998bea41dd0d46f38bb963fc8"
tokenBridgeAddrStr := "0x0290fb167208af455bb137780163b7b7a9a10c16" //nolint:gosec
tokenBridgeAddr, err := vaa.StringToAddress(tokenBridgeAddrStr)
require.NoError(t, err)

gov.setDayLengthInMinutes(24 * 60)
err = gov.setChainForTesting(vaa.ChainIDEthereum, tokenBridgeAddrStr, 1000000, 100000)
require.NoError(t, err)
err = gov.setTokenForTesting(vaa.ChainIDEthereum, tokenAddrStr, "WETH", 1774.62)
require.NoError(t, err)

now := time.Now()
messageTimestamp := now.Add(-5) // 5 seconds ago

// message that, when processed, should exceed the big transfer size
msg := common.MessagePublication{
TxHash: hashFromString("0x06f541f5ecfc43407c31587aa6ac3a689e8960f36dc23c332db5510dfc6a4063"),
Timestamp: messageTimestamp,
Nonce: uint32(1),
Sequence: uint64(3),
EmitterChain: vaa.ChainIDEthereum,
EmitterAddress: tokenBridgeAddr,
ConsistencyLevel: uint8(32),
Payload: buildMockTransferPayloadBytes(1,
vaa.ChainIDEthereum,
tokenAddrStr,
vaa.ChainIDPolygon,
toAddrStr,
100,
),
}

canPost, err := gov.ProcessMsgForTime(&msg, now)
require.NoError(t, err)
assert.Equal(t, false, canPost)

msg.MessageIDString()

// check that the enqueued vaa's release date is now + 1 day
expectedReleaseTime := uint32(now.Add(24 * time.Hour).Unix())
enqueuedVaas := gov.GetEnqueuedVAAs()
assert.Equal(t, len(enqueuedVaas), 1)
assert.Equal(t, enqueuedVaas[0].ReleaseTime, expectedReleaseTime)

// the release timer gets reset to 5 days
_, err = gov.resetReleaseTimerForTime(msg.MessageIDString(), now, 5)
require.NoError(t, err)

// check that the enqueued vaa's release date is now + 5 days
enqueuedVaas = gov.GetEnqueuedVAAs()
assert.Equal(t, len(enqueuedVaas), 1)
expectedReleaseTime = uint32(now.Add(5 * 24 * time.Hour).Unix())
assert.Equal(t, enqueuedVaas[0].ReleaseTime, expectedReleaseTime)

}

func TestLargeTransactionGetsEnqueuedAndReleasedWhenTheTimerExpires(t *testing.T) {
ctx := context.Background()
gov, err := newChainGovernorForTest(ctx)
Expand Down Expand Up @@ -1481,7 +1544,7 @@ func TestLargeTransactionGetsEnqueuedAndReleasedWhenTheTimerExpires(t *testing.T
assert.Equal(t, 1, len(gov.msgsSeen))

// But then the operator resets the release time.
_, err = gov.resetReleaseTimerForTime(msg3.MessageIDString(), now)
_, err = gov.resetReleaseTimerForTime(msg3.MessageIDString(), now, 1)
require.NoError(t, err)

// So now, 12 hours later the big transaction is enqueued, it still won't get released.
Expand Down
Loading

0 comments on commit 627faa7

Please sign in to comment.