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

Address todo comments #367

Merged
merged 3 commits into from
May 12, 2022
Merged

Address todo comments #367

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1918eaa
Address or doc todos in issues
alpe May 9, 2022
818df72
Fix fees test
alpe May 11, 2022
3dc0447
Review feedback
alpe May 12, 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
2 changes: 1 addition & 1 deletion app/export.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func (app *TgradeApp) ExportAppStateAndValidators(
}
// as if they could withdraw from the start of the next block
ctx := app.NewContext(true, tmproto.Header{Height: app.LastBlockHeight()}).
WithBlockTime(time.Now().UTC()) // todo (Alex): check if there is any way to get the last block time
WithBlockTime(time.Now().UTC())

// We export at last height + 1, because that's the height at which
// Tendermint will start InitChain.
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ replace (
// Use the cosmos-flavored keyring library
github.com/99designs/keyring => github.com/cosmos/keyring v1.1.7-0.20210622111912-ef00f8ac3d76
// Fix upstream GHSA-h395-qcrw-5vmq vulnerability.
// TODO Remove it: https://github.com/cosmos/cosmos-sdk/issues/10409
// Remove it: https://github.com/cosmos/cosmos-sdk/issues/10409
Copy link
Contributor

Choose a reason for hiding this comment

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

Up to you, but I would leave the TODO label. It's good for quickly finding / flagging these things.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This todo is not actionable currently.

github.com/gin-gonic/gin => github.com/gin-gonic/gin v1.7.0
// latest grpc doesn't work with with our modified proto compiler, so we need to enforce
// the following version across all dependencies.
Expand Down
36 changes: 25 additions & 11 deletions testing/fee_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ import (
"fmt"
"testing"

"github.com/stretchr/testify/assert"

poetypes "github.com/confio/tgrade/x/poe/types"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/stretchr/testify/require"
"github.com/tidwall/gjson"
Expand Down Expand Up @@ -46,25 +50,35 @@ func TestFeeDistribution(t *testing.T) {
sut.StartChain(t)

cli := NewTgradeCli(t, sut, verbose)
cli.FundAddress(cli.AddKey("myFatFingerKey"), "20000000utgd")
cli.FundAddress(cli.AddKey("myFatFingerKey"), "200000000utgd")
oldBalances := make([]int64, sut.nodesCount)
for i := 0; i < sut.nodesCount; i++ {
oldBalances[i] = cli.QueryBalance(cli.GetKeyAddr(fmt.Sprintf("node%d", i)), "utgd")
}

// when
const anyContract = "testing/contract/hackatom.wasm.gzip"
txResult := cli.CustomCommand("tx", "wasm", "store", anyContract, "--from=myFatFingerKey", "--gas=1500000", "--fees=20000000utgd")
txResult := cli.CustomCommand("tx", "wasm", "store", anyContract, "--from=myFatFingerKey", "--gas=1500000", "--fees=200000000utgd")
RequireTxSuccess(t, txResult)
AwaitValsetEpochCompleted(t) // so that fees are distributed

// TODO: no more fees are distributed. Rather they are held in a new contract to be withdrawn.
// DO proper fix in issue #156, so we can query the pending stake. For now I will disable
// then
//for i := 0; i < sut.nodesCount; i++ {
// newBalance := cli.QueryBalance(cli.GetKeyAddr(fmt.Sprintf("node%d", i)), "utgd")
// diff := newBalance - oldBalances[i]
// t.Logf("Block rewards: %d\n", diff)
// assert.LessOrEqualf(t, int64(20000000/sut.nodesCount), diff, "got diff: %d (before %d after %d)", diff, oldBalances[i], newBalance)
//}
// and rewards claimed
alpe marked this conversation as resolved.
Show resolved Hide resolved
distAddr := cli.GetPoEContractAddress(poetypes.PoEContractTypeDistribution.String())
for i := 0; i < sut.nodesCount; i++ {
rsp := cli.Execute(distAddr, `{"withdraw_rewards":{}}`, fmt.Sprintf("node%d", i))
RequireTxSuccess(t, rsp)
}
engAddr := cli.GetPoEContractAddress(poetypes.PoEContractTypeEngagement.String())
for i := 0; i < sut.nodesCount; i++ {
rsp := cli.Execute(engAddr, `{"withdraw_rewards":{}}`, fmt.Sprintf("node%d", i))
RequireTxSuccess(t, rsp)
}
// then balance contains rewards
// 200000000 * 47.5% *(1/ 4 + 1/10) = 33250000 # 1/4 is reserved for all vals, 1/10 is reserved for all EPs
const expMinRevenue int64 = 33250000
for i := 0; i < sut.nodesCount; i++ {
newBalance := cli.QueryBalance(cli.GetKeyAddr(fmt.Sprintf("node%d", i)), "utgd")
diff := newBalance - oldBalances[i]
assert.LessOrEqualf(t, expMinRevenue, diff, "node %d got diff: %d (before %d after %d)", i, diff, oldBalances[i], newBalance)
}
}
2 changes: 1 addition & 1 deletion x/poe/abci.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func EndBlocker(parentCtx sdk.Context, k endBlockKeeper) []abci.ValidatorUpdate
diff, err = contract.CallEndBlockWithValidatorUpdate(ctx, contractAddr, k)
if err != nil {
logger.Error("validator set update failed", "cause", err, "contract-address", contractAddr, "position", pos)
panic(err) // this breaks consensus
panic(err) // this will crash the node as panics are not recovered
}
commit()
if len(diff) != 0 {
Expand Down
3 changes: 0 additions & 3 deletions x/poe/bootstrap.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,6 @@ func BootstrapPoEContracts(ctx sdk.Context, k wasmtypes.ContractOpsKeeper, tk tw
LeftGroup: engagementContractAddr.String(),
RightGroup: stakeContractAddr.String(),
PreAuthsSlashing: 1,
// TODO: allow to configure the other types.
// We need to analyze benchmarks and discuss first.
// This maintains same behavior
FunctionType: contract.MixerFunction{
GeometricMean: &struct{}{},
},
Expand Down
1 change: 0 additions & 1 deletion x/poe/contract/tg4_stake.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,6 @@ func (v StakeContractAdapter) QueryStakedAmount(ctx sdk.Context, opAddr sdk.AccA
}

// QueryStakingUnbonding query PoE staking contract for unbonded self delegations
// TODO: add pagination support here!
func (v StakeContractAdapter) QueryStakingUnbonding(ctx sdk.Context, opAddr sdk.AccAddress) ([]stakingtypes.UnbondingDelegationEntry, error) {
if v.addressLookupErr != nil {
return nil, v.addressLookupErr
Expand Down
8 changes: 3 additions & 5 deletions x/poe/contract/tgrade_oc_proposals.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,9 @@ type OCProposalResponse struct {
Proposal OversightProposal `json:"proposal"`
Status ProposalStatus `json:"status"`
CreatedBy string `json:"created_by"`
// TODO: clarify this format
// Expires EXP `json:"expires"`
Rules VotingRules `json:"rules"`
TotalPoints uint64 `json:"total_points"`
Votes Votes `json:"votes"`
Rules VotingRules `json:"rules"`
TotalPoints uint64 `json:"total_points"`
Votes Votes `json:"votes"`
}

type OCProposalListResponse struct {
Expand Down
3 changes: 0 additions & 3 deletions x/poe/contract/tgrade_valset.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,9 +191,6 @@ type ValsetEpochResponse struct {
LastUpdateTime uint64 `json:"last_update_time"`
// The last time we updated the validator set - block height
LastUpdateHeight uint64 `json:"last_update_height"`
// TODO: add this if you want it, not in current code
// Seconds (UTC UNIX time) of next timestamp that will trigger a validator recalculation
// NextUpdateTime int `json:"next_update_time"`
}

type OperatorResponse struct {
Expand Down
2 changes: 0 additions & 2 deletions x/poe/module.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,6 @@ func (b AppModuleBasic) ValidateGenesis(cdc codec.JSONCodec, txEncodingConfig cl
if err := cdc.UnmarshalJSON(bz, &data); err != nil {
return fmt.Errorf("failed to unmarshal %s genesis state: %w", types.ModuleName, err)
}
// todo: add PoE validation

return types.ValidateGenesis(data, txEncodingConfig.TxJSONDecoder())
}

Expand Down
4 changes: 0 additions & 4 deletions x/poe/stakingadapter/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,11 @@ func (s StakingAdapter) GetBondedValidatorsByPower(ctx sdk.Context) []stakingtyp
}

func (s StakingAdapter) GetAllDelegatorDelegations(ctx sdk.Context, delegator sdk.AccAddress) []stakingtypes.Delegation {
// todo: return self delegation of validator
log(ctx, "GetAllDelegatorDelegations")
return nil
}

func (s StakingAdapter) GetDelegation(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress) (delegation stakingtypes.Delegation, found bool) {
// todo: handle for validator operator's self delegation
log(ctx, "GetDelegation")
return
}
Expand Down Expand Up @@ -108,7 +106,6 @@ func (s StakingAdapter) Unjail(ctx sdk.Context, address sdk.ConsAddress) {
}

func (s StakingAdapter) Delegation(ctx sdk.Context, address sdk.AccAddress, address2 sdk.ValAddress) stakingtypes.DelegationI {
// todo: handle for validator operator's self delegation
log(ctx, "Delegation")
return nil
}
Expand Down Expand Up @@ -138,7 +135,6 @@ func (s2 StakingAdapter) TotalBondedTokens(ctx sdk.Context) sdk.Int {
}

func (s2 StakingAdapter) IterateDelegations(ctx sdk.Context, delegator sdk.AccAddress, fn func(index int64, delegation stakingtypes.DelegationI) (stop bool)) {
// todo: handle for validators self delegations
log(ctx, "IterateDelegations")
}

Expand Down
2 changes: 1 addition & 1 deletion x/twasm/abci.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func BeginBlocker(parentCtx sdk.Context, k abciKeeper, b abci.RequestBeginBlock)

msgBz, err := json.Marshal(msg)
if err != nil {
panic(err) // todo (reviewer): this will break consensus
alpe marked this conversation as resolved.
Show resolved Hide resolved
panic(err) // this will crash the node as panics are not recovered
}
logger := keeper.ModuleLogger(parentCtx)
k.IteratePrivilegedContractsByType(parentCtx, types.PrivilegeTypeBeginBlock, func(pos uint8, contractAddr sdk.AccAddress) bool {
Expand Down
6 changes: 1 addition & 5 deletions x/twasm/contract/incoming_msgs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,6 @@ func (p *ExecuteGovProposal) unpackInterfaces(unpacker codectypes.AnyUnpacker) e
if p.Proposal.RegisterUpgrade.UpgradedClientState != nil {
return sdkerrors.ErrInvalidRequest.Wrap("upgrade logic for IBC has been moved to the IBC module")
}
// todo (Alex): check how a client update is done now. this also needs to go into the contracts
// case p.Proposal.IBCClientUpdate != nil:
// return p.Proposal.IBCClientUpdate.UnpackInterfaces(unpacker)
}
return err
}
Expand Down Expand Up @@ -181,7 +178,6 @@ func (p *GovProposal) UnmarshalJSON(b []byte) error {
}
result.IBCClientUpdate = &ibcclienttypes.ClientUpdateProposal{
SubjectClientId: proxy.ClientId,
// todo (Alex): SubstituteClientId: proxy.ClientId,
}
return nil
},
Expand All @@ -202,7 +198,7 @@ func (p *GovProposal) UnmarshalJSON(b []byte) error {
return nil
},
"migrate_contract": func(b []byte) error {
proxy := struct { // todo: better use wasmvmtypes.MigrateMsg when names match
proxy := struct { // custom type as not name compatible with wasmvmtypes.MigrateMsg
Contract string `json:"contract"`
CodeID uint64 `json:"code_id"`
Msg []byte `json:"migrate_msg"`
Expand Down
1 change: 0 additions & 1 deletion x/twasm/contract/incoming_msgs_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ func TestGetProposalContent(t *testing.T) {
Title: "foo",
Description: "bar",
SubjectClientId: "myClientID",
// todo (Alex):fix also in contracts!! SubstituteClientId: ib,
},
skipValidateBasic: true,
},
Expand Down