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

Commit

Permalink
failure to submit ops should count towards the delete cycles threshold (
Browse files Browse the repository at this point in the history 
closes #498) (#499)

* 1 - add check to delete all offers on failure from SubmitOps, conditionally logPrefix of "(async)" inside the method

* 2 - fix issue of unintentional reuse of err variable in sdex.go#submit which caused hiding the error
  • Loading branch information
@nikhilsaraf
nikhilsaraf authored Sep 8, 2020
1 parent 9b2dc57 commit 6434510
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 30 deletions.
20 changes: 10 additions & 10 deletions plugins/sdex.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -446,14 +446,14 @@ func (sdex *SDEX) sign(tx *txnbuild.Transaction) (string, error) {
}

func (sdex *SDEX) submit(txeB64 string, asyncCallback func(hash string, e error), asyncMode bool) {
resp, err := sdex.API.SubmitTransactionXDR(txeB64)
if err != nil {
if herr, ok := errors.Cause(err).(*horizonclient.Error); ok {
resp, e := sdex.API.SubmitTransactionXDR(txeB64)
if e != nil {
if herr, ok := errors.Cause(e).(*horizonclient.Error); ok {
var rcs *hProtocol.TransactionResultCodes
rcs, err = herr.ResultCodes()
if err != nil {
log.Printf("(async) error: no result codes from horizon: %s\n", err)
sdex.invokeAsyncCallback(asyncCallback, "", err, asyncMode)
rcs, e2 := herr.ResultCodes()
if e2 != nil {
log.Printf("(async) error: no result codes from horizon: %s\n", e2)
sdex.invokeAsyncCallback(asyncCallback, "", e2, asyncMode)
return
}
if rcs.TransactionCode == "tx_bad_seq" {
Expand All @@ -462,9 +462,9 @@ func (sdex *SDEX) submit(txeB64 string, asyncCallback func(hash string, e error)
}
log.Println("(async) error: result code details: tx code =", rcs.TransactionCode, ", opcodes =", rcs.OperationCodes)
} else {
log.Printf("(async) error: tx failed for unknown reason, error message: %s\n", err)
log.Printf("(async) error: tx failed for unknown reason, error message: %s\n", e)
}
sdex.invokeAsyncCallback(asyncCallback, "", err, asyncMode)
sdex.invokeAsyncCallback(asyncCallback, "", e, asyncMode)
return
}

Expand All @@ -476,7 +476,7 @@ func (sdex *SDEX) submit(txeB64 string, asyncCallback func(hash string, e error)
sdex.invokeAsyncCallback(asyncCallback, resp.Hash, nil, asyncMode)
}

func (sdex *SDEX) invokeAsyncCallback(asyncCallback func(hash string, err error), hash string, err error, asyncMode bool) {
func (sdex *SDEX) invokeAsyncCallback(asyncCallback func(hash string, e error), hash string, err error, asyncMode bool) {
if asyncCallback == nil {
return
}
Expand Down
50 changes: 30 additions & 20 deletions trader/trader.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,19 +140,23 @@ func (t *Trader) Start() {
}

// deletes all offers for the bot (not all offers on the account)
func (t *Trader) deleteAllOffers() {
func (t *Trader) deleteAllOffers(isAsync bool) {
logPrefix := ""
if isAsync {
logPrefix = "(async) "
}
if t.deleteCyclesThreshold < 0 {
log.Printf("not deleting any offers because deleteCyclesThreshold is negative\n")
log.Printf("%snot deleting any offers because deleteCyclesThreshold is negative\n", logPrefix)
return
}

t.deleteCycles++
if t.deleteCycles <= t.deleteCyclesThreshold {
log.Printf("not deleting any offers, deleteCycles (=%d) needs to exceed deleteCyclesThreshold (=%d)\n", t.deleteCycles, t.deleteCyclesThreshold)
log.Printf("%snot deleting any offers, deleteCycles (=%d) needs to exceed deleteCyclesThreshold (=%d)\n", logPrefix, t.deleteCycles, t.deleteCyclesThreshold)
return
}

log.Printf("deleting all offers, num. continuous update cycles with errors (including this one): %d; (deleteCyclesThreshold to be exceeded=%d)\n", t.deleteCycles, t.deleteCyclesThreshold)
log.Printf("%sdeleting all offers, num. continuous update cycles with errors (including this one): %d; (deleteCyclesThreshold to be exceeded=%d)\n", logPrefix, t.deleteCycles, t.deleteCyclesThreshold)
dOps := []txnbuild.Operation{}
dOps = append(dOps, t.sdex.DeleteAllOffers(t.sellingAOffers)...)
t.sellingAOffers = []hProtocol.Offer{}
Expand All @@ -161,23 +165,23 @@ func (t *Trader) deleteAllOffers() {

// LOH-3 - we want to guarantee that the bot crashes if the errors exceed deleteCyclesThreshold, so we start a new thread with a sleep timer to crash the bot as a safety
defer func() {
log.Printf("started thread to crash bot in 1 minute as a fallback (to respect deleteCyclesThreshold)\n")
log.Printf("%sstarted thread to crash bot in 1 minute as a fallback (to respect deleteCyclesThreshold)\n", logPrefix)
time.Sleep(time.Minute)
log.Fatalf("bot should have crashed by now (programmer error?), crashing\n")
log.Fatalf("%sbot should have crashed by now (programmer error?), crashing\n", logPrefix)
}()

log.Printf("created %d operations to delete offers\n", len(dOps))
log.Printf("%screated %d operations to delete offers\n", logPrefix, len(dOps))
if len(dOps) > 0 {
// to delete offers the submitMode doesn't matter, so use api.SubmitModeBoth as the default
e := t.exchangeShim.SubmitOps(api.ConvertOperation2TM(dOps), api.SubmitModeBoth, func(hash string, e error) {
log.Fatalf("...deleted %d offers, exiting (asyncCallback: hash=%s, e=%v)", len(dOps), hash, e)
log.Fatalf("(async) ...deleted %d offers, exiting (asyncCallback: hash=%s, e=%v)", len(dOps), hash, e)
})
if e != nil {
log.Fatalf("continuing to exit after showing error during submission of delete offer ops: %s", e)
log.Fatalf("%scontinuing to exit after showing error during submission of delete offer ops: %s", logPrefix, e)
return
}
} else {
log.Fatalf("...nothing to delete, exiting")
log.Fatalf("%s...nothing to delete, exiting", logPrefix)
}
}

Expand Down Expand Up @@ -301,7 +305,7 @@ func (t *Trader) update() bool {
e := t.synchronizeFetchBalancesOffersTrades()
if e != nil {
log.Println(e)
t.deleteAllOffers()
t.deleteAllOffers(false)
return false
}

Expand All @@ -320,15 +324,15 @@ func (t *Trader) update() bool {
t.sdex.IEIF().LogAllLiabilities(t.assetBase, t.assetQuote)
if e != nil {
log.Println(e)
t.deleteAllOffers()
t.deleteAllOffers(false)
return false
}

// strategy has a chance to set any state it needs
e = t.strategy.PreUpdate(t.maxAssetA, t.maxAssetB, t.trustAssetA, t.trustAssetB)
if e != nil {
log.Println(e)
t.deleteAllOffers()
t.deleteAllOffers(false)
return false
}

Expand All @@ -341,7 +345,7 @@ func (t *Trader) update() bool {
e = t.exchangeShim.SubmitOps(pruneOps, api.SubmitModeBoth, nil)
if e != nil {
log.Println(e)
t.deleteAllOffers()
t.deleteAllOffers(false)
return false
}
}
Expand All @@ -355,7 +359,7 @@ func (t *Trader) update() bool {
t.sdex.IEIF().LogAllLiabilities(t.assetBase, t.assetQuote)
if e != nil {
log.Println(e)
t.deleteAllOffers()
t.deleteAllOffers(false)
return false
}

Expand All @@ -366,7 +370,7 @@ func (t *Trader) update() bool {
log.Println(e)
log.Printf("liabilities (force recomputed) after encountering an error after a call to UpdateWithOps\n")
t.sdex.IEIF().RecomputeAndLogCachedLiabilities(t.assetBase, t.assetQuote)
t.deleteAllOffers()
t.deleteAllOffers(false)
return false
}

Expand All @@ -375,25 +379,31 @@ func (t *Trader) update() bool {
ops, e = filter.Apply(ops, t.sellingAOffers, t.buyingAOffers)
if e != nil {
log.Printf("error in filter index %d: %s\n", i, e)
t.deleteAllOffers()
t.deleteAllOffers(false)
return false
}
}

log.Printf("created %d operations to update existing offers\n", len(ops))
if len(ops) > 0 {
e = t.exchangeShim.SubmitOps(api.ConvertOperation2TM(ops), t.submitMode, nil)
e = t.exchangeShim.SubmitOps(api.ConvertOperation2TM(ops), t.submitMode, func(hash string, e error) {
// if there is an error we want it to count towards the delete cycles threshold, so run the check
if e != nil {
t.deleteAllOffers(true)
}
})

if e != nil {
log.Println(e)
t.deleteAllOffers()
t.deleteAllOffers(false)
return false
}
}

e = t.strategy.PostUpdate()
if e != nil {
log.Println(e)
t.deleteAllOffers()
t.deleteAllOffers(false)
return false
}

Expand Down

0 comments on commit 6434510

Please sign in to comment.