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

Add RequiredFee field to Check/DeliverResult #357

Merged
merged 11 commits into from
Feb 27, 2019
Merged

Add RequiredFee field to Check/DeliverResult #357

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
86ade8c
Add RequiredFee field, import cycle
ethanfrey Feb 26, 2019
5d7413e
Add up RequiredFees in batch - test once compiles
ethanfrey Feb 26, 2019
e6c50f7
Extract coins into own package
ethanfrey Feb 26, 2019
5a04b52
Fix many references to x.Coin to coin.Coin
ethanfrey Feb 26, 2019
8fb1966
Fix remaining references to x.Coin, etc
ethanfrey Feb 26, 2019
ab27913
make build passes
ethanfrey Feb 26, 2019
85029a2
Fill all imports in tests
ethanfrey Feb 26, 2019
ef13c76
Merge pull request #358 from iov-one/extract-coin-package
ethanfrey Feb 26, 2019
50cbb5d
Added unit test for batch fee combination
ethanfrey Feb 27, 2019
25b3309
Batch handles combining product fees with missing product fees
ethanfrey Feb 27, 2019
06fd274
Updated docstring for RequiredFee
ethanfrey Feb 27, 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
5 changes: 3 additions & 2 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,12 @@ ifndef $(shell command -v prototool help > /dev/null)
endif
prototool lint

protoc: protodocs
# protoc: protodocs
protoc:
protoc --gogofaster_out=. app/*.proto
protoc --gogofaster_out=. coin/*.proto
protoc --gogofaster_out=. crypto/*.proto
protoc --gogofaster_out=. orm/*.proto
protoc --gogofaster_out=. x/*.proto
# Note, you must include -I=./vendor when compiling files that use gogoprotobuf extensions
protoc --gogofaster_out=. -I=. -I=./vendor -I=$(GOPATH)/src x/nft/*.proto
protoc --gogofaster_out=. -I=. -I=./vendor -I=$(GOPATH)/src cmd/bnsd/x/nft/username/*.proto
Expand Down
26 changes: 20 additions & 6 deletions abci.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package weave
import (
"fmt"

"github.com/iov-one/weave/coin"
"github.com/iov-one/weave/errors"
abci "github.com/tendermint/tendermint/abci/types"
"github.com/tendermint/tendermint/libs/common"
Expand Down Expand Up @@ -35,11 +36,19 @@ func CheckOrError(result CheckResult, err error, debug bool) abci.ResponseCheckT
// DeliverResult captures any non-error abci result
// to make sure people use error for error cases
type DeliverResult struct {
Data []byte
Log string
Diff []abci.ValidatorUpdate
Tags []common.KVPair
GasUsed int64 // unused
// Data is a machine-parseable return value, like id of created entity
Data []byte
// Log is human-readable informational string
Log string
// RequiredFee can set an custom fee that must be paid for this transaction to be allowed to run.
// This is enforced in cash.DynamicFeeDecorator
Copy link
Contributor

Choose a reason for hiding this comment

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

I think changing the phrasing from this is enforced to this might be enforced by for example might explain better. Although today this field is added specifically for the use by the cash.DynamicFeeDecorator, anyone should be able to use it. Otherwise it sounds like this is a hard dependency. 💅

I love the comments though. They explain a lot! 🏅

RequiredFee coin.Coin
// Diff, if present, will apply to the Validator set in tendermint next block
Diff []abci.ValidatorUpdate
// Tags, if present, will be used by tendermint to index and search the transaction history
Tags []common.KVPair
// GasUsed is currently unused field until effects in tendermint are clear
GasUsed int64
}

// ToABCI converts our internal type into an abci response
Expand All @@ -54,8 +63,13 @@ func (d DeliverResult) ToABCI() abci.ResponseDeliverTx {
// CheckResult captures any non-error abci result
// to make sure people use error for error cases
type CheckResult struct {
// Data is a machine-parseable return value, like id of created entity
Data []byte
Log string
// Log is human-readable informational string
Log string
// RequiredFee can set an custom fee that must be paid for this transaction to be allowed to run.
// This is enforced in cash.DynamicFeeDecorator
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 similar to the discussion on https://github.com/iov-one/weave/pull/357/files#diff-dc931a8b7120348333d7b68dce1ca6ffR44

RequiredFee coin.Coin
// GasAllocated is the maximum units of work we allow this tx to perform
GasAllocated int64
// GasPayment is the total fees for this tx (or other source of payment)
Expand Down
34 changes: 17 additions & 17 deletions cmd/bcpd/app/app_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import (

"github.com/iov-one/weave"
"github.com/iov-one/weave/app"
"github.com/iov-one/weave/coin"
"github.com/iov-one/weave/crypto"
"github.com/iov-one/weave/x"
"github.com/iov-one/weave/x/batch"
"github.com/iov-one/weave/x/cash"
"github.com/iov-one/weave/x/escrow"
Expand All @@ -34,7 +34,7 @@ func TestSendTx(t *testing.T) {
// Query for my balance
key := cash.NewBucket().DBKey(mainAccount.address())
queryAndCheckWallet(t, false, myApp, "/", key, cash.Set{
Coins: x.Coins{
Coins: coin.Coins{
{Ticker: "ETH", Whole: 50000},
{Ticker: "FRNK", Whole: 1234},
},
Expand Down Expand Up @@ -67,7 +67,7 @@ func TestSendTx(t *testing.T) {

queryBalance := func() {
queryAndCheckWallet(t, false, myApp, "/", key, cash.Set{
Coins: x.Coins{
Coins: coin.Coins{
{Ticker: "ETH", Whole: 30000},
{Ticker: "FRNK", Whole: 1234},
},
Expand Down Expand Up @@ -103,7 +103,7 @@ func TestQuery(t *testing.T) {
// Query for new balances
key := cash.NewBucket().DBKey(mainAccount.address())
queryAndCheckWallet(t, false, myApp, "/", key, cash.Set{
Coins: x.Coins{
Coins: coin.Coins{
{Ticker: "ETH", Whole: 48000},
{Ticker: "FRNK", Whole: 1234},
},
Expand All @@ -112,14 +112,14 @@ func TestQuery(t *testing.T) {
// make sure money arrived safely
key2 := cash.NewBucket().DBKey(addr2)
queryAndCheckWallet(t, false, myApp, "/", key2, cash.Set{
Coins: x.Coins{
Coins: coin.Coins{
{Ticker: "ETH", Whole: 2000},
},
})

// make sure other paths also get this value....
queryAndCheckWallet(t, false, myApp, "/wallets", addr2, cash.Set{
Coins: x.Coins{
Coins: coin.Coins{
{Ticker: "ETH", Whole: 2000},
},
})
Expand Down Expand Up @@ -221,24 +221,24 @@ func newMultisigTestApp(t require.TestingT, chainID string, contracts []*contrac
func withWalletAppState(t require.TestingT, accounts []*account) string {
type wallet struct {
Address weave.Address `json:"address"`
Coins x.Coins `json:"coins"`
Coins coin.Coins `json:"coins"`
}
state := struct {
Cash []wallet `json:"cash"`
Gconf map[string]interface{} `json:"gconf"`
}{
Gconf: map[string]interface{}{
cash.GconfCollectorAddress: "fake-collector-address",
cash.GconfMinimalFee: x.Coin{}, // no fee
cash.GconfMinimalFee: coin.Coin{}, // no fee
},
}

for _, acc := range accounts {
state.Cash = append(state.Cash, wallet{
Address: acc.address(),
Coins: x.Coins{
&x.Coin{Whole: 50000, Ticker: "ETH"},
&x.Coin{Whole: 1234, Ticker: "FRNK"},
Coins: coin.Coins{
&coin.Coin{Whole: 50000, Ticker: "ETH"},
&coin.Coin{Whole: 1234, Ticker: "FRNK"},
},
})
}
Expand Down Expand Up @@ -313,7 +313,7 @@ func sendToken(t require.TestingT, baseApp app.BaseApp, chainID string, height i
msg := &cash.SendMsg{
Src: from,
Dest: to,
Amount: &x.Coin{
Amount: &coin.Coin{
Whole: amount,
Ticker: ticker,
},
Expand All @@ -329,7 +329,7 @@ func sendToken(t require.TestingT, baseApp app.BaseApp, chainID string, height i

// make sure money arrived safely
queryAndCheckWallet(t, false, baseApp, "/wallets", to, cash.Set{
Coins: x.Coins{
Coins: coin.Coins{
{Ticker: ticker, Whole: amount},
},
})
Expand All @@ -343,7 +343,7 @@ func sendBatch(t require.TestingT, fail bool, baseApp app.BaseApp, chainID strin
msg := &cash.SendMsg{
Src: from,
Dest: to,
Amount: &x.Coin{
Amount: &coin.Coin{
Whole: amount,
Ticker: ticker,
},
Expand Down Expand Up @@ -382,7 +382,7 @@ func sendBatch(t require.TestingT, fail bool, baseApp app.BaseApp, chainID strin

// make sure money arrived only for successful batch
queryAndCheckWallet(t, fail, baseApp, "/wallets", to, cash.Set{
Coins: x.Coins{
Coins: coin.Coins{
{Ticker: ticker, Whole: checkAmount},
},
})
Expand Down Expand Up @@ -493,7 +493,7 @@ func makeSendTx(t require.TestingT, chainID string, sender, receiver *account, t
msg := &cash.SendMsg{
Src: sender.address(),
Dest: receiver.address(),
Amount: &x.Coin{
Amount: &coin.Coin{
Whole: amount,
Ticker: ticker,
},
Expand All @@ -520,7 +520,7 @@ func makeSendTxMultisig(t require.TestingT, chainID string, sender, receiver *co
msg := &cash.SendMsg{
Src: sender.address(),
Dest: receiver.address(),
Amount: &x.Coin{
Amount: &coin.Coin{
Whole: amount,
Ticker: ticker,
},
Expand Down
10 changes: 5 additions & 5 deletions cmd/bcpd/app/examples.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package app

import (
"github.com/iov-one/weave/coin"
"github.com/iov-one/weave/commands"
"github.com/iov-one/weave/crypto"
"github.com/iov-one/weave/x"
"github.com/iov-one/weave/x/cash"
"github.com/iov-one/weave/x/namecoin"
"github.com/iov-one/weave/x/sigs"
Expand All @@ -13,9 +13,9 @@ import (
func Examples() []commands.Example {
wallet := &namecoin.Wallet{
Name: "example",
Coins: []*x.Coin{
&x.Coin{Whole: 50000, Ticker: "ETH"},
&x.Coin{Whole: 150, Fractional: 567000, Ticker: "BTC"},
Coins: []*coin.Coin{
&coin.Coin{Whole: 50000, Ticker: "ETH"},
&coin.Coin{Whole: 150, Fractional: 567000, Ticker: "BTC"},
},
}

Expand All @@ -32,7 +32,7 @@ func Examples() []commands.Example {
}

dst := crypto.GenPrivKeyEd25519().PublicKey().Address()
amt := x.NewCoin(250, 0, "ETH")
amt := coin.NewCoin(250, 0, "ETH")
msg := &cash.SendMsg{
Amount: &amt,
Dest: dst,
Expand Down
4 changes: 2 additions & 2 deletions cmd/bcpd/app/init.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ import (

"github.com/iov-one/weave"
"github.com/iov-one/weave/app"
"github.com/iov-one/weave/coin"
"github.com/iov-one/weave/crypto"
"github.com/iov-one/weave/gconf"
"github.com/iov-one/weave/x"
"github.com/iov-one/weave/x/cash"
"github.com/iov-one/weave/x/currency"
"github.com/iov-one/weave/x/distribution"
Expand All @@ -28,7 +28,7 @@ func GenInitOptions(args []string) (json.RawMessage, error) {
ticker := "IOV"
if len(args) > 0 {
ticker = args[0]
if !x.IsCC(ticker) {
if !coin.IsCC(ticker) {
return nil, fmt.Errorf("Invalid ticker %s", ticker)
}
}
Expand Down
6 changes: 3 additions & 3 deletions cmd/bcpd/commands/init_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ import (

"github.com/iov-one/weave"
"github.com/iov-one/weave/cmd/bcpd/app"
"github.com/iov-one/weave/coin"
"github.com/iov-one/weave/commands/server"
"github.com/iov-one/weave/x"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/tendermint/tendermint/libs/log"
Expand All @@ -37,7 +37,7 @@ func TestInit(t *testing.T) {
State struct {
Cash []struct {
Address weave.Address
Coins x.Coins
Coins coin.Coins
}
} `json:"app_state"`
}
Expand All @@ -50,7 +50,7 @@ func TestInit(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, weave.Address(want), wallet.Address)
require.Equal(t, 1, len(wallet.Coins), "Genesis: %s", bz)
assert.Equal(t, &x.Coin{Ticker: args[0], Whole: 123456789}, wallet.Coins[0])
assert.Equal(t, &coin.Coin{Ticker: args[0], Whole: 123456789}, wallet.Coins[0])
}

// setupConfig creates a homedir to run inside,
Expand Down
16 changes: 8 additions & 8 deletions cmd/bnsd/app/app_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import (
weaveApp "github.com/iov-one/weave/app"
"github.com/iov-one/weave/cmd/bnsd/app"
"github.com/iov-one/weave/cmd/bnsd/app/testdata/fixtures"
"github.com/iov-one/weave/coin"
"github.com/iov-one/weave/crypto"
"github.com/iov-one/weave/x"
"github.com/iov-one/weave/x/cash"
"github.com/iov-one/weave/x/multisig"
"github.com/iov-one/weave/x/sigs"
Expand All @@ -28,7 +28,7 @@ func TestApp(t *testing.T) {
// Query for my balance
key := cash.NewBucket().DBKey(appFixture.GenesisKeyAddress)
queryAndCheckAccount(t, myApp, "/", key, cash.Set{
Coins: x.Coins{
Coins: coin.Coins{
{Ticker: "ETH", Whole: 50000},
{Ticker: "FRNK", Whole: 1234},
},
Expand Down Expand Up @@ -63,7 +63,7 @@ func TestApp(t *testing.T) {

// Query for new balances (same query, new state)
queryAndCheckAccount(t, myApp, "/", key, cash.Set{
Coins: x.Coins{
Coins: coin.Coins{
{Ticker: "ETH", Whole: 48000},
{Ticker: "FRNK", Whole: 1234},
},
Expand All @@ -72,7 +72,7 @@ func TestApp(t *testing.T) {
// make sure money arrived safely
key2 := cash.NewBucket().DBKey(addr2)
queryAndCheckAccount(t, myApp, "/", key2, cash.Set{
Coins: x.Coins{
Coins: coin.Coins{
{
Ticker: "ETH",
Whole: 2000,
Expand All @@ -82,12 +82,12 @@ func TestApp(t *testing.T) {

// make sure other paths also get this value....
queryAndCheckAccount(t, myApp, "/wallets", addr2, cash.Set{
Coins: x.Coins{{Ticker: "ETH", Whole: 2000}},
Coins: coin.Coins{{Ticker: "ETH", Whole: 2000}},
})

// make sure other paths also get this value....
queryAndCheckAccount(t, myApp, "/wallets?prefix", addr2[:15], cash.Set{
Coins: x.Coins{
Coins: coin.Coins{
{Ticker: "ETH", Whole: 2000},
},
})
Expand Down Expand Up @@ -140,7 +140,7 @@ func sendToken(t *testing.T, baseApp weaveApp.BaseApp, chainID string, height in
msg := &cash.SendMsg{
Src: from,
Dest: to,
Amount: &x.Coin{Whole: amount, Ticker: ticker},
Amount: &coin.Coin{Whole: amount, Ticker: ticker},
Memo: memo,
}
tx := &app.Tx{
Expand All @@ -149,7 +149,7 @@ func sendToken(t *testing.T, baseApp weaveApp.BaseApp, chainID string, height in
}
res := signAndCommit(t, baseApp, tx, signers, chainID, height)
// make sure money arrived safely
queryAndCheckAccount(t, baseApp, "/wallets", to, cash.Set{Coins: x.Coins{{Ticker: ticker, Whole: amount}}})
queryAndCheckAccount(t, baseApp, "/wallets", to, cash.Set{Coins: coin.Coins{{Ticker: ticker, Whole: amount}}})
return res
}

Expand Down
10 changes: 5 additions & 5 deletions cmd/bnsd/app/examples.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ import (
"fmt"

"github.com/iov-one/weave/cmd/bnsd/x/nft/username"
"github.com/iov-one/weave/coin"
"github.com/iov-one/weave/commands"
"github.com/iov-one/weave/crypto"
"github.com/iov-one/weave/x"
"github.com/iov-one/weave/x/cash"
"github.com/iov-one/weave/x/namecoin"
"github.com/iov-one/weave/x/nft"
Expand All @@ -17,9 +17,9 @@ import (
func Examples() []commands.Example {
wallet := &namecoin.Wallet{
Name: "example",
Coins: []*x.Coin{
&x.Coin{Whole: 50000, Ticker: "ETH"},
&x.Coin{Whole: 150, Fractional: 567000, Ticker: "BTC"},
Coins: []*coin.Coin{
&coin.Coin{Whole: 50000, Ticker: "ETH"},
&coin.Coin{Whole: 150, Fractional: 567000, Ticker: "BTC"},
},
}

Expand All @@ -37,7 +37,7 @@ func Examples() []commands.Example {
}

dst := crypto.GenPrivKeyEd25519().PublicKey().Address()
amt := x.NewCoin(250, 0, "ETH")
amt := coin.NewCoin(250, 0, "ETH")
msg := &cash.SendMsg{
Amount: &amt,
Dest: dst,
Expand Down
Loading