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.

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

Rebroadcast unconfirmed transactions upon .Connect #1676

Merged
merged 1 commit into from
Sep 18, 2019

Conversation

spooktheducks
Copy link
Contributor

@spooktheducks spooktheducks commented Sep 16, 2019

core/store/orm/orm_test.go Outdated Show resolved Hide resolved

_, err := txm.SendRawTx(attempt.SignedRawTx)
if err != nil {
logger.Warnf("Failed to rebroadcast tx %v: %v", attempt.Hash, err)
Copy link
Contributor

Choose a reason for hiding this comment

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

@se3000 @j16r We have a situation here where transaction attempts that have been confirmed while we've been offline might return an error on rebroadcast. We've opted to simply warn on this situation because we don't want to deter the reconnect. Please let me know if you have any thoughts on this.

func (orm *ORM) UnconfirmedTxAttempts() ([]models.TxAttempt, error) {
var items []models.TxAttempt
err := orm.DB.
Where("confirmed = false").
Copy link
Contributor

Choose a reason for hiding this comment

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

@j16r @se3000
Technically, this confirmed bool flag is only set when Safe, not on the first confirmation via a receipt. We've opted to use this anyways and rebroadcast txs that might be confirmed but not safe, but warn in that scenario, since that scenario could happen while we're offline anyways. ie:

  1. tx A is sent, but unconfirmed
  2. We go offline, but it gets confirmed
  3. We reconnect and rebroadcast.

Ideally, we would want to rename this column safe and have a separate column confirmed to also identify the mere act of confirmation, or the reception of a tx receipt. That's been captured here: https://www.pivotaltracker.com/story/show/168536671

@spooktheducks spooktheducks merged commit 1b6b870 into develop Sep 18, 2019
@se3000 se3000 deleted the feature/rebroadcast-unconfirmed-tx branch September 20, 2019 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants