Skip to content
This repository has been archived by the owner on Apr 23, 2024. It is now read-only.

sdk/agent: add a first happy path test #255

Merged
merged 40 commits into from
Aug 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
4f75e29
sdk/agent: make open/pay/close ops consistently async
leighmcculloch Aug 21, 2021
0d63856
Merge branch 'main' into issue224
leighmcculloch Aug 21, 2021
764dca2
sdk/agent: add event handlers (wip)
leighmcculloch Aug 21, 2021
442b88b
1
leighmcculloch Aug 23, 2021
392b170
sdk/agent: move logic for determining initiator to a func
leighmcculloch Aug 23, 2021
ad74b3b
Merge branch 'smll' into issue226
leighmcculloch Aug 23, 2021
fe5db7e
wip
leighmcculloch Aug 23, 2021
1969226
sdk/agent: add todo to remove logic
leighmcculloch Aug 23, 2021
ff00ff0
Merge branch 'todoit' into issue226
leighmcculloch Aug 23, 2021
89a7d30
wip
leighmcculloch Aug 24, 2021
3326e5e
Merge branch 'main' into issue226
leighmcculloch Aug 24, 2021
32a8da7
wip
leighmcculloch Aug 24, 2021
4474a63
sdk/agent: move channel creation into open process
leighmcculloch Aug 24, 2021
e50696b
make the initiator the open proposer
leighmcculloch Aug 24, 2021
a7973dc
Merge branch 'moveinit' into issue226
leighmcculloch Aug 24, 2021
5214249
Reduce changes
leighmcculloch Aug 24, 2021
85d1a53
on error in more cases
leighmcculloch Aug 24, 2021
3430c58
simplify
leighmcculloch Aug 24, 2021
487092f
simplify
leighmcculloch Aug 24, 2021
b1d0248
consistent error message
leighmcculloch Aug 24, 2021
2a3f943
log error
leighmcculloch Aug 24, 2021
4232962
group
leighmcculloch Aug 24, 2021
77dc195
Merge branch 'moveinit' into issue226
leighmcculloch Aug 24, 2021
6ddb567
event handling
leighmcculloch Aug 24, 2021
d653992
remove defer
leighmcculloch Aug 24, 2021
c514fab
wip
leighmcculloch Aug 24, 2021
7a03c0e
wip
leighmcculloch Aug 26, 2021
dd55482
remove todos
leighmcculloch Aug 26, 2021
9388e31
test passing
leighmcculloch Aug 26, 2021
f8743b4
logger resilience
leighmcculloch Aug 26, 2021
6c2750b
Revert "logger resilience"
leighmcculloch Aug 26, 2021
e9f9c38
ignore logs
leighmcculloch Aug 26, 2021
22a4aa4
Merge branch 'main' into issue226
leighmcculloch Aug 26, 2021
1d73c0e
sdk/agent: repair failing payment due to stale balance data
leighmcculloch Aug 26, 2021
ae1584b
Merge branch 'underfundedagent' into issue226
leighmcculloch Aug 26, 2021
19d17c7
strip event handlers from change
leighmcculloch Aug 26, 2021
f23ad1e
remove event handlers
leighmcculloch Aug 26, 2021
a8fbd45
fix fmt
leighmcculloch Aug 26, 2021
077926d
Merge branch 'main' into firsttest
leighmcculloch Aug 26, 2021
4f48c24
Merge branch 'main' into firsttest
leighmcculloch Aug 27, 2021
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
30 changes: 18 additions & 12 deletions sdk/agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,20 +254,26 @@ func (a *Agent) Close() error {
return nil
}

func (a *Agent) loop() {
var err error
func (a *Agent) receive() error {
recv := msg.NewDecoder(io.TeeReader(a.conn, a.LogWriter))
send := msg.NewEncoder(io.MultiWriter(a.conn, a.LogWriter))
m := msg.Message{}
err := recv.Decode(&m)
if err != nil {
return fmt.Errorf("reading and decoding: %v\n", err)
}
err = a.handle(m, send)
if err != nil {
return fmt.Errorf("handling message: %v\n", err)
}
return nil
}

func (a *Agent) receiveLoop() {
for {
m := msg.Message{}
err = recv.Decode(&m)
if err != nil {
fmt.Fprintf(a.LogWriter, "error reading: %v\n", err)
break
}
err = a.handle(m, send)
err := a.receive()
if err != nil {
fmt.Fprintf(a.LogWriter, "error handling message: %v\n", err)
fmt.Fprintf(a.LogWriter, "error receiving: %v\n", err)
}
}
}
Expand All @@ -276,11 +282,11 @@ func (a *Agent) handle(m msg.Message, send *msg.Encoder) error {
fmt.Fprintf(a.LogWriter, "handling %v\n", m.Type)
handler := handlerMap[m.Type]
if handler == nil {
return fmt.Errorf("unrecognized message type %v", m.Type)
return fmt.Errorf("handling message %d: unrecognized message type", m.Type)
}
err := handler(a, m, send)
if err != nil {
return fmt.Errorf("handling message type %v: %w", m.Type, err)
return fmt.Errorf("handling message %d: %w", m.Type, err)
}
return nil
}
Expand Down
170 changes: 170 additions & 0 deletions sdk/agent/agent_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
package agent

import (
"bytes"
"io"
"testing"
"time"

"github.com/stellar/experimental-payment-channels/sdk/state"
"github.com/stellar/go/keypair"
"github.com/stellar/go/network"
"github.com/stellar/go/txnbuild"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

type sequenceNumberCollector func(accountID *keypair.FromAddress) (int64, error)
Copy link
Contributor

Choose a reason for hiding this comment

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

❓ what's the reason for re-creating these functions instead of using the exported ones?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm defining a type, which can be a function, that satisfies each of the interfaces so that in tests I can write quick little functions inline for them. This is a common pattern most known for its use in the stdlib's http package, namely the http.HandleFunc type that is a function that implements the http.Handle interface.

There are a couple proposals being debated at the moment to make it possible in Go to convert a function to a single-function interface: golang/go#21670, golang/go#$47487. If either proposal ever gets accepted it'll make it unnecessary to define these types in tests.

Copy link
Contributor

@acharb acharb Aug 27, 2021

Choose a reason for hiding this comment

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

oops I asked that and then figured it out and thought I deleted the question. Thanks for the additional info though makes sense!


func (f sequenceNumberCollector) GetSequenceNumber(accountID *keypair.FromAddress) (int64, error) {
return f(accountID)
}

type balanceCollectorFunc func(accountID *keypair.FromAddress, asset state.Asset) (int64, error)

func (f balanceCollectorFunc) GetBalance(accountID *keypair.FromAddress, asset state.Asset) (int64, error) {
return f(accountID, asset)
}

type submitterFunc func(tx *txnbuild.Transaction) error

func (f submitterFunc) SubmitTx(tx *txnbuild.Transaction) error {
return f(tx)
}

func TestAgent_openPaymentClose(t *testing.T) {
localEscrow := keypair.MustRandom()
localSigner := keypair.MustRandom()
remoteEscrow := keypair.MustRandom()
remoteSigner := keypair.MustRandom()

// Setup the local agent.
localVars := struct {
submittedTx *txnbuild.Transaction
}{}
localAgent := &Agent{
ObservationPeriodTime: 20 * time.Second,
ObservationPeriodLedgerGap: 1,
MaxOpenExpiry: 5 * time.Minute,
NetworkPassphrase: network.TestNetworkPassphrase,
SequenceNumberCollector: sequenceNumberCollector(func(accountID *keypair.FromAddress) (int64, error) {
return 1, nil
}),
BalanceCollector: balanceCollectorFunc(func(accountID *keypair.FromAddress, asset state.Asset) (int64, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

💭 thoughts on returning the passed in int64? Or possibly another source instead of hardcoding 🤔 may come in handy on future tests where the balance will change and we want test that

also could be a change in the PR that requires that, so maybe not this one more I think about it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we can implement that in whatever test needs it. The way I've implement that is to set a new BalanceCollector at each point I want a new value.

I'd do that either by setting a new function that returns a new hardcoded value, or by changing the balanceCollectorFunc to a fixedBalanceCollector that is an int64 that returns itself. Either way works.

return 100_0000000, nil
}),
Submitter: submitterFunc(func(tx *txnbuild.Transaction) error {
localVars.submittedTx = tx
return nil
}),
EscrowAccountKey: localEscrow.FromAddress(),
EscrowAccountSigner: localSigner,
LogWriter: io.Discard,
}

// Setup the remote agent.
remoteVars := struct {
submittedTx *txnbuild.Transaction
}{}
remoteAgent := &Agent{
ObservationPeriodTime: 20 * time.Second,
ObservationPeriodLedgerGap: 1,
MaxOpenExpiry: 5 * time.Minute,
NetworkPassphrase: network.TestNetworkPassphrase,
SequenceNumberCollector: sequenceNumberCollector(func(accountID *keypair.FromAddress) (int64, error) {
return 1, nil
}),
BalanceCollector: balanceCollectorFunc(func(accountID *keypair.FromAddress, asset state.Asset) (int64, error) {
return 100_0000000, nil
}),
Submitter: submitterFunc(func(tx *txnbuild.Transaction) error {
remoteVars.submittedTx = tx
return nil
}),
EscrowAccountKey: remoteEscrow.FromAddress(),
EscrowAccountSigner: remoteSigner,
LogWriter: io.Discard,
}

// Connect the two agents.
type ReadWriter struct {
io.Reader
io.Writer
}
localMsgs := bytes.Buffer{}
remoteMsgs := bytes.Buffer{}
localAgent.conn = ReadWriter{
Reader: &remoteMsgs,
Writer: &localMsgs,
}
remoteAgent.conn = ReadWriter{
Reader: &localMsgs,
Writer: &remoteMsgs,
}
err := localAgent.hello()
require.NoError(t, err)
err = remoteAgent.receive()
require.NoError(t, err)
err = remoteAgent.hello()
require.NoError(t, err)
err = localAgent.receive()
require.NoError(t, err)

// Open the channel.
err = localAgent.Open()
require.NoError(t, err)
err = remoteAgent.receive()
require.NoError(t, err)
err = localAgent.receive()
require.NoError(t, err)

// Expect the open tx to have been submitted.
openTx, err := localAgent.channel.OpenTx()
require.NoError(t, err)
assert.Equal(t, openTx, localVars.submittedTx)
localVars.submittedTx = nil

// Make a payment.
err = localAgent.Payment("50.0")
require.NoError(t, err)
err = remoteAgent.receive()
require.NoError(t, err)
err = localAgent.receive()
require.NoError(t, err)

// Make another payment.
err = remoteAgent.Payment("20.0")
require.NoError(t, err)
err = localAgent.receive()
require.NoError(t, err)
err = remoteAgent.receive()
require.NoError(t, err)

// Expect no txs to have been submitted for payments.
assert.Nil(t, localVars.submittedTx)
assert.Nil(t, remoteVars.submittedTx)

// Declare the close, and start negotiating for an early close.
err = localAgent.DeclareClose()
require.NoError(t, err)

// Expect the declaration tx to have been submitted.
localDeclTx, _, err := localAgent.channel.CloseTxs()
require.NoError(t, err)
assert.Equal(t, localDeclTx, localVars.submittedTx)

// Receive the declaration at the remote and complete negotiation.
err = remoteAgent.receive()
require.NoError(t, err)
err = localAgent.receive()
require.NoError(t, err)

// Expect the close tx to have been submitted.
_, localCloseTx, err := localAgent.channel.CloseTxs()
require.NoError(t, err)
_, remoteCloseTx, err := remoteAgent.channel.CloseTxs()
require.NoError(t, err)
assert.Equal(t, localCloseTx, remoteCloseTx)
assert.Equal(t, localCloseTx, localVars.submittedTx)
assert.Equal(t, remoteCloseTx, remoteVars.submittedTx)
}
4 changes: 2 additions & 2 deletions sdk/agent/tcp.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func (a *Agent) ServeTCP(addr string) error {
if err != nil {
return fmt.Errorf("sending hello: %w", err)
}
go a.loop()
go a.receiveLoop()
return nil
}

Expand All @@ -42,6 +42,6 @@ func (a *Agent) ConnectTCP(addr string) error {
if err != nil {
return fmt.Errorf("sending hello: %w", err)
}
go a.loop()
go a.receiveLoop()
return nil
}