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

update: added a fix to the boost issue #661

Merged
merged 4 commits into from May 4, 2023
Merged

Conversation

tobi-bams
Copy link
Contributor

No description provided.

@tobi-bams tobi-bams linked an issue Apr 20, 2023 that may be closed by this pull request
@Evanfeenstra
Copy link
Contributor

@tobi-bams I dont think this is the best solution. Using the global var FAILED_TO_SEND_ID could make things get complicated, like if two tribe messages are sending at the same time (from different parts of the code).

I would solve it like this:

  • check if there is a contactId that is equal to the realSatsContactId
  • if so, move it to the front of the array
const realSatsIndex = contactIds.findIndex(cid=> cid===realSatsContactId)
if (realSatsIndex>0) {
  contactIds.unshift(contactIds.splice(realSatsIndex, 1)[0])
}
  • instead of await asyncForEach(contactIds..., use a for (const contactId of contactIds) {}
  • now, instead of "return" when you want to skip a contactId (for example if (destkey === skipPubKey)) you can use "continue" to skip the iteration. And now if the first one fails, you can break so that the rest of the contactIds are not tried.

@Evanfeenstra
Copy link
Contributor

@tobi-bams can u write a test for failed boosts?

Where tribe admin tries to send more sats then they have. It should create a failed Message

@Evanfeenstra
Copy link
Contributor

Then we need to think about the next problem: tribe member boosts another member… the sats make it to the admin ok, but then the forwarded boost fails! In this case, the one who sent the original boost should see the failure, and also they should have their sats returned. Make sense?

Lets do that in a seperate PR after you write a test for the first simple case

@Evanfeenstra Evanfeenstra merged commit 69fb454 into master May 4, 2023
33 checks passed
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.

boost issues
2 participants