Skip to content

Commit

Permalink
Merge pull request #426 from iov-one/revenue_distrubtion_address_issu…
Browse files Browse the repository at this point in the history
…e_425

Fix revenue account computation
  • Loading branch information
husio authored Mar 20, 2019
2 parents 5f050bf + 96c6354 commit dd44350
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 16 deletions.
5 changes: 4 additions & 1 deletion cmd/bnsd/scenarios/distribution_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,10 @@ func TestRevenueDistribution(t *testing.T) {
revenueID := weave.Address(resp.Response.DeliverTx.GetData())
t.Logf("new revenue stream id: %s", revenueID)

revenueAddress := distribution.RevenueAccount(revenueID)
revenueAddress, err := distribution.RevenueAccount(revenueID)
if err != nil {
t.Fatalf("cannot create a revenue account for %d: %s", revenueID, err)
}
t.Logf("new revenue stream account: %s", revenueAddress)

delayForRateLimits()
Expand Down
2 changes: 1 addition & 1 deletion cmd/bnsd/scenarios/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func TestMain(m *testing.M) {
flag.Parse()
multiSigContract = multisig.MultiSigCondition(weavetest.SequenceID(1))
escrowContract = escrow.Condition(weavetest.SequenceID(1))
distrContractAddr = distribution.RevenueAccount(weavetest.SequenceID(1))
distrContractAddr, _ = distribution.RevenueAccount(weavetest.SequenceID(1))

alice = derivePrivateKey(*hexSeed, *derivationPath)
logger.Error("Loaded Alice key", "addressID", alice.PublicKey().Address())
Expand Down
6 changes: 5 additions & 1 deletion cmd/lsaddr/lsaddr.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,11 @@ var converters = map[string]converter{
return multisig.MultiSigCondition(seq(i)).Address()
},
"distribution": func(i int) weave.Address {
return distribution.RevenueAccount(seq(i))
a, err := distribution.RevenueAccount(seq(i))
if err != nil {
panic(err)
}
return a
},
}

Expand Down
13 changes: 10 additions & 3 deletions x/distribution/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,11 @@ func (h *distributeHandler) Deliver(ctx weave.Context, db weave.KVStore, tx weav
return res, err
}

if err := distribute(db, h.ctrl, RevenueAccount(msg.RevenueID), rev.Recipients); err != nil {
racc, err := RevenueAccount(msg.RevenueID)
if err != nil {
return res, errors.Wrap(err, "invalid revenue account")
}
if err := distribute(db, h.ctrl, racc, rev.Recipients); err != nil {
return res, errors.Wrap(err, "cannot distribute")
}
return res, nil
Expand Down Expand Up @@ -187,12 +191,15 @@ func (h *resetRevenueHandler) Deliver(ctx weave.Context, db weave.KVStore, tx we
if err != nil {
return res, err
}

racc, err := RevenueAccount(msg.RevenueID)
if err != nil {
return res, errors.Wrap(err, "invalid revenue account")
}
// Before updating the revenue all funds must be distributed. Only a
// revenue with no funds can be updated, so that recipients trust us.
// Otherwise an admin could change who receives the money without the
// previously selected recepients ever being paid.
if err := distribute(db, h.ctrl, RevenueAccount(msg.RevenueID), rev.Recipients); err != nil {
if err := distribute(db, h.ctrl, racc, rev.Recipients); err != nil {
return res, errors.Wrap(err, "cannot distribute")
}

Expand Down
25 changes: 17 additions & 8 deletions x/distribution/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,15 @@ func TestHandlers(t *testing.T) {
ctrl := cash.NewController(cashBucket)
RegisterRoutes(rt, auth, ctrl)

revenueAccount := func(revID uint64) weave.Address {
t.Helper()
a, err := RevenueAccount(weavetest.SequenceID(revID))
if err != nil {
t.Fatal(err)
}
return a
}

// In below cases, weavetest.SequenceID(1) is used - this is the
// address of the first revenue instance created. Sequence is reset for
// each test case.
Expand Down Expand Up @@ -103,7 +112,7 @@ func TestHandlers(t *testing.T) {
},
"weights are normalized during distribution": {
prepareAccounts: []account{
{address: RevenueAccount(weavetest.SequenceID(1)), coins: coin.Coins{coin.NewCoinp(0, 7, "BTC")}},
{address: revenueAccount(1), coins: coin.Coins{coin.NewCoinp(0, 7, "BTC")}},
},
wantAccounts: []account{
// All funds must be transferred to the only recipient.
Expand Down Expand Up @@ -163,10 +172,10 @@ func TestHandlers(t *testing.T) {
},
"revenue with an account but without enough funds": {
prepareAccounts: []account{
{address: RevenueAccount(weavetest.SequenceID(1)), coins: coin.Coins{coin.NewCoinp(0, 1, "BTC")}},
{address: revenueAccount(1), coins: coin.Coins{coin.NewCoinp(0, 1, "BTC")}},
},
wantAccounts: []account{
{address: RevenueAccount(weavetest.SequenceID(1)), coins: coin.Coins{coin.NewCoinp(0, 1, "BTC")}},
{address: revenueAccount(1), coins: coin.Coins{coin.NewCoinp(0, 1, "BTC")}},
},
actions: []action{
{
Expand Down Expand Up @@ -195,10 +204,10 @@ func TestHandlers(t *testing.T) {
},
"distribute revenue with a leftover funds": {
prepareAccounts: []account{
{address: RevenueAccount(weavetest.SequenceID(1)), coins: coin.Coins{coin.NewCoinp(0, 7, "BTC")}},
{address: revenueAccount(1), coins: coin.Coins{coin.NewCoinp(0, 7, "BTC")}},
},
wantAccounts: []account{
{address: RevenueAccount(weavetest.SequenceID(1)), coins: coin.Coins{coin.NewCoinp(0, 1, "BTC")}},
{address: revenueAccount(1), coins: coin.Coins{coin.NewCoinp(0, 1, "BTC")}},
{address: addr1, coins: coin.Coins{coin.NewCoinp(0, 2, "BTC")}},
{address: addr2, coins: coin.Coins{coin.NewCoinp(0, 4, "BTC")}},
},
Expand Down Expand Up @@ -229,10 +238,10 @@ func TestHandlers(t *testing.T) {
},
"distribute revenue with an account holding various tickers": {
prepareAccounts: []account{
{address: RevenueAccount(weavetest.SequenceID(1)), coins: coin.Coins{coin.NewCoinp(0, 3, "BTC"), coin.NewCoinp(0, 7, "ETH")}},
{address: revenueAccount(1), coins: coin.Coins{coin.NewCoinp(0, 3, "BTC"), coin.NewCoinp(0, 7, "ETH")}},
},
wantAccounts: []account{
{address: RevenueAccount(weavetest.SequenceID(1)), coins: coin.Coins{coin.NewCoinp(0, 1, "ETH")}},
{address: revenueAccount(1), coins: coin.Coins{coin.NewCoinp(0, 1, "ETH")}},
{address: addr1, coins: coin.Coins{coin.NewCoinp(0, 1, "BTC"), coin.NewCoinp(0, 2, "ETH")}},
{address: addr2, coins: coin.Coins{coin.NewCoinp(0, 2, "BTC"), coin.NewCoinp(0, 4, "ETH")}},
},
Expand Down Expand Up @@ -263,7 +272,7 @@ func TestHandlers(t *testing.T) {
},
"updating a revenue is distributing the collected funds first": {
prepareAccounts: []account{
{address: RevenueAccount(weavetest.SequenceID(1)), coins: coin.Coins{coin.NewCoinp(0, 3, "BTC")}},
{address: revenueAccount(1), coins: coin.Coins{coin.NewCoinp(0, 3, "BTC")}},
},
wantAccounts: []account{
{address: addr1, coins: coin.Coins{coin.NewCoinp(0, 1, "BTC")}},
Expand Down
8 changes: 6 additions & 2 deletions x/distribution/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,12 @@ func (b *RevenueBucket) Create(db weave.KVStore, rev *Revenue) (orm.Object, erro

// RevenueAccount returns an account address that is holding funds of a revenue
// with given ID.
func RevenueAccount(revenueID []byte) weave.Address {
return weave.NewCondition("distribution", "revenue", revenueID).Address()
func RevenueAccount(revenueID []byte) (weave.Address, error) {
c := weave.NewCondition("dist", "revenue", revenueID)
if err := c.Validate(); err != nil {
return nil, errors.Wrap(err, "condition")
}
return c.Address(), nil
}

// Save persists the state of a given revenue entity.
Expand Down

0 comments on commit dd44350

Please sign in to comment.