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.

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

feat(cosmic-swingset): Leave inbound actions in vstorage #7115

Merged
merged 12 commits into from
Mar 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ replace google.golang.org/grpc => google.golang.org/grpc v1.33.2
replace github.com/tendermint/tendermint => github.com/agoric-labs/tendermint v0.34.23-alpha.agoric.3

// We need a fork of cosmos-sdk until all of the differences are merged.
replace github.com/cosmos/cosmos-sdk => github.com/agoric-labs/cosmos-sdk v0.45.11-alpha.agoric.1
replace github.com/cosmos/cosmos-sdk => github.com/agoric-labs/cosmos-sdk v0.45.11-alpha.agoric.1.0.20230320225042-2109765fd835

replace github.com/cosmos/gaia/v7 => github.com/Agoric/ag0/v7 v7.0.2-alpha.agoric.1

Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ github.com/Zilliqa/gozilliqa-sdk v1.2.1-0.20201201074141-dd0ecada1be6/go.mod h1:
github.com/adlio/schema v1.3.3 h1:oBJn8I02PyTB466pZO1UZEn1TV5XLlifBSyMrmHl/1I=
github.com/aead/siphash v1.0.1/go.mod h1:Nywa3cDsYNNK3gaciGTWPwHt0wlpNV15vwmswBAUSII=
github.com/afex/hystrix-go v0.0.0-20180502004556-fa1af6a1f4f5/go.mod h1:SkGFH1ia65gfNATL8TAiHDNxPzPdmEL5uirI2Uyuz6c=
github.com/agoric-labs/cosmos-sdk v0.45.11-alpha.agoric.1 h1:3GHpCatyGBaZDoSr9k6Rd5/5QnNghkzxgLfkDsPDBPk=
github.com/agoric-labs/cosmos-sdk v0.45.11-alpha.agoric.1/go.mod h1:fdXvzy+wmYB+W+N139yb0+szbT7zAGgUjmxm5DBrjto=
github.com/agoric-labs/cosmos-sdk v0.45.11-alpha.agoric.1.0.20230320225042-2109765fd835 h1:Mmw52cHAUNwtaNXpk7b3lTeoCRd5Vw9Fdrly5ABIxCA=
github.com/agoric-labs/cosmos-sdk v0.45.11-alpha.agoric.1.0.20230320225042-2109765fd835/go.mod h1:fdXvzy+wmYB+W+N139yb0+szbT7zAGgUjmxm5DBrjto=
github.com/agoric-labs/cosmos-sdk/ics23/go v0.8.0-alpha.agoric.1 h1:2jvHI/2d+psWAZy6FQ0vXJCHUtfU3ZbbW+pQFL04arQ=
github.com/agoric-labs/cosmos-sdk/ics23/go v0.8.0-alpha.agoric.1/go.mod h1:E45NqnlpxGnpfTWL/xauN7MRwEE28T4Dd4uraToOaKg=
github.com/agoric-labs/tendermint v0.34.23-alpha.agoric.3 h1:aq6F1r3RQkKUYNeMNjRxgGn3dayvKnDK/R6gQF0WoFs=
Expand Down
21 changes: 5 additions & 16 deletions golang/cosmos/x/swingset/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package swingset
import (
// "fmt"
// "os"
"encoding/json"
"time"

"github.com/cosmos/cosmos-sdk/telemetry"
Expand All @@ -23,10 +22,6 @@ type beginBlockAction struct {
Params types.Params `json:"params"`
}

type beginBlockResult struct {
QueueAllowed []types.QueueSize `json:"queue_allowed"`
}

type endBlockAction struct {
Type string `json:"type"`
BlockHeight int64 `json:"blockHeight"`
Expand All @@ -50,20 +45,14 @@ func BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock, keeper Keeper) erro
ChainID: ctx.ChainID(),
Params: keeper.GetParams(ctx),
}
out, err := keeper.BlockingSend(ctx, action)
_, err := keeper.BlockingSend(ctx, action)
// fmt.Fprintf(os.Stderr, "BEGIN_BLOCK Returned from SwingSet: %s, %v\n", out, err)

if out != "" {
var result beginBlockResult
err := json.Unmarshal([]byte(out), &result)
if err != nil {
panic(err)
}
state := keeper.GetState(ctx)
state.QueueAllowed = result.QueueAllowed
keeper.SetState(ctx, state)
if err != nil {
panic(err)
}

err = keeper.UpdateQueueAllowed(ctx)

return err
}

Expand Down
110 changes: 93 additions & 17 deletions golang/cosmos/x/swingset/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@ import (
"encoding/json"
"errors"
"fmt"
stdlog "log"
"math"
"strconv"
"math/big"

"github.com/tendermint/tendermint/libs/log"

"github.com/cosmos/cosmos-sdk/baseapp"
"github.com/cosmos/cosmos-sdk/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
bankkeeper "github.com/cosmos/cosmos-sdk/x/bank/keeper"
Expand All @@ -32,10 +34,30 @@ const (
StoragePathBundles = "bundles"
)

const MaxUint53 = 9007199254740991 // Number.MAX_SAFE_INTEGER = 2**53 - 1
// 2 ** 256 - 1
var MaxSDKInt = sdk.NewIntFromBigInt(new(big.Int).Sub(new(big.Int).Exp(big.NewInt(2), big.NewInt(256), nil), big.NewInt(1)))

const stateKey string = "state"

// Contextual information about the message source of an action on the queue.
// This context should be unique per actionQueueRecord.
type actionContext struct {
// The block height in which the corresponding action was enqueued
BlockHeight int64 `json:"blockHeight"`
// The hash of the cosmos transaction that included the message
// If the action didn't result from a transaction message, a substitute value
// may be used. For example the VBANK_BALANCE_UPDATE actions use `x/vbank`.
TxHash string `json:"txHash"`
// The index of the message within the transaction. If the action didn't
// result from a cosmos transaction, a number should be chosen to make the
// actionContext unique. (for example a counter per block and source module).
MsgIdx int `json:"msgIdx"`
}
type actionQueueRecord struct {
Action vm.Jsonable `json:"action"`
Context actionContext `json:"context"`
}

// Keeper maintains the link to data vstorage and exposes getter/setter methods for the various parts of the state machine
type Keeper struct {
storeKey sdk.StoreKey
Expand Down Expand Up @@ -87,40 +109,55 @@ func NewKeeper(
// The actionQueue's format is documented by `makeChainQueue` in
// `packages/cosmic-swingset/src/make-queue.js`.
func (k Keeper) PushAction(ctx sdk.Context, action vm.Jsonable) error {
bz, err := json.Marshal(action)
txHash, txHashOk := ctx.Context().Value(baseapp.TxHashContextKey).(string)
if !txHashOk {
txHash = "unknown"
}
msgIdx, msgIdxOk := ctx.Context().Value(baseapp.TxMsgIdxContextKey).(int)
if !txHashOk || !msgIdxOk {
stdlog.Printf("error while extracting context for action %q\n", action)
}
record := actionQueueRecord{Action: action, Context: actionContext{BlockHeight: ctx.BlockHeight(), TxHash: txHash, MsgIdx: msgIdx}}
bz, err := json.Marshal(record)
if err != nil {
return err
}

// Get the current queue tail, defaulting to zero if its vstorage doesn't exist.
// The `tail` is the value of the next index to be inserted
tail, err := k.actionQueueIndex(ctx, "tail")
if err != nil {
return err
}

// JS uses IEEE 754 floats so avoid overflowing integers
if tail == MaxUint53 {
if tail.Equal(MaxSDKInt) {
return errors.New(StoragePathActionQueue + " overflow")
}
nextTail := tail.Add(sdk.NewInt(1))

// Set the vstorage corresponding to the queue entry for the current tail.
path := StoragePathActionQueue + "." + strconv.FormatUint(tail, 10)
path := StoragePathActionQueue + "." + tail.String()
k.vstorageKeeper.SetStorage(ctx, vstoragetypes.NewStorageEntry(path, string(bz)))

// Update the tail to point to the next available entry.
path = StoragePathActionQueue + ".tail"
k.vstorageKeeper.SetStorage(ctx, vstoragetypes.NewStorageEntry(path, strconv.FormatUint(tail+1, 10)))
k.vstorageKeeper.SetStorage(ctx, vstoragetypes.NewStorageEntry(path, nextTail.String()))
return nil
}

func (k Keeper) actionQueueIndex(ctx sdk.Context, name string) (uint64, error) {
index := uint64(0)
var err error
indexEntry := k.vstorageKeeper.GetEntry(ctx, StoragePathActionQueue+"."+name)
if indexEntry.HasData() {
index, err = strconv.ParseUint(indexEntry.StringValue(), 10, 64)
func (k Keeper) actionQueueIndex(ctx sdk.Context, position string) (sdk.Int, error) {
// Position should be either "head" or "tail"
path := StoragePathActionQueue + "." + position
indexEntry := k.vstorageKeeper.GetEntry(ctx, path)
if !indexEntry.HasData() {
return sdk.NewInt(0), nil
}
return index, err

index, ok := sdk.NewIntFromString(indexEntry.StringValue())
if !ok {
return index, fmt.Errorf("couldn't parse %s as Int: %s", path, indexEntry.StringValue())
}
return index, nil
}

func (k Keeper) ActionQueueLength(ctx sdk.Context) (int32, error) {
Expand All @@ -132,11 +169,50 @@ func (k Keeper) ActionQueueLength(ctx sdk.Context) (int32, error) {
if err != nil {
return 0, err
}
size := tail - head
if size > math.MaxInt32 {
// The tail index is exclusive
size := tail.Sub(head)
if !size.IsInt64() {
return 0, fmt.Errorf("%s size too big: %s", StoragePathActionQueue, size)
}

int64Size := size.Int64()
if int64Size > math.MaxInt32 {
return math.MaxInt32, nil
}
return int32(size), nil
return int32(int64Size), nil
}

func (k Keeper) UpdateQueueAllowed(ctx sdk.Context) error {
params := k.GetParams(ctx)
inboundQueueMax, found := types.QueueSizeEntry(params.QueueMax, types.QueueInbound)
if !found {
return errors.New("could not find max inboundQueue size in params")
}
inboundMempoolQueueMax := inboundQueueMax / 2
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a separate entry: types.QueueInboundMempool. Please correct any comment or doc which says it's always half of QueueInbound.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't understand, which docs or comment says that?

Copy link
Contributor

Choose a reason for hiding this comment

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

Let me turn my request in to a question instead - what made you think that the inbound mempool queue max should be half of the inbound queue max?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's what the existing behavior that I'm porting from JS is.

While we have a params.QueueMax value for QueueInbound, we don't have an explicit param value for QueueInboundMempool, which is hardcoded and documented as 50% of the QueueInbound value.

However we do need to compute a a value for the QueueInboundMempool of the state.QueueAllowed. That value is indeed only going to be 50% of the allowed QueueInbound if the actionQueue is empty. However I don't believe there is any claim anywhere that the allowed QueueInboundMempool is always 50% of the allowed QueueInbound.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, okay, I get it now.


inboundQueueSize, err := k.ActionQueueLength(ctx)
if err != nil {
return err
}

var inboundQueueAllowed int32
if inboundQueueMax > inboundQueueSize {
inboundQueueAllowed = inboundQueueMax - inboundQueueSize
}

var inboundMempoolQueueAllowed int32
if inboundMempoolQueueMax > inboundQueueSize {
inboundMempoolQueueAllowed = inboundMempoolQueueMax - inboundQueueSize
}

state := k.GetState(ctx)
state.QueueAllowed = []types.QueueSize{
{Key: types.QueueInbound, Size_: inboundQueueAllowed},
{Key: types.QueueInboundMempool, Size_: inboundMempoolQueueAllowed},
}
k.SetState(ctx, state)

return nil
}

// BlockingSend sends a message to the controller and blocks the Golang process
Expand Down
8 changes: 8 additions & 0 deletions golang/cosmos/x/vbank/vbank.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
package vbank

import (
"context"
"encoding/json"
"fmt"
stdlog "log"
"sort"

"github.com/Agoric/agoric-sdk/golang/cosmos/vm"
"github.com/cosmos/cosmos-sdk/baseapp"
sdk "github.com/cosmos/cosmos-sdk/types"
)

Expand Down Expand Up @@ -241,5 +243,11 @@ func (ch portHandler) Receive(ctx *vm.ControllerContext, str string) (ret string
}

func (am AppModule) PushAction(ctx sdk.Context, action vm.Jsonable) error {
// vbank actions are not triggered by a swingset message in a transaction, so we need to
// synthesize unique context information.
// We use a fixed placeholder value for the txHash context, and can simply use `0` for the
// message index as there is only one such action per block.
ctx = ctx.WithContext(context.WithValue(ctx.Context(), baseapp.TxHashContextKey, "x/vbank"))
mhofman marked this conversation as resolved.
Show resolved Hide resolved
ctx = ctx.WithContext(context.WithValue(ctx.Context(), baseapp.TxMsgIdxContextKey, 0))
return am.keeper.PushAction(ctx, action)
}
Loading