Fix challenge api tx confirming #9526
Conversation
🦋 Changeset detectedLatest commit: cf06185 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
rickyrombo
left a comment
rickyrombo
left a comment
There was a problem hiding this comment.
Great find! This will help with the evaluate failures for sure
| return await Promise.all( | ||
| transactions.map((t) => this.rewardManager.sendTransaction(t)) | ||
| transactions.map((t) => { | ||
| return { transactionSignature: this.rewardManager.sendTransaction(t) } |
There was a problem hiding this comment.
this.rewardManager.sendTransaction(t) should be awaited here
| excludeOwners: string[] | ||
| logger?: LoggerService | ||
| }) { | ||
| }): Promise<Array<{ transactionSignature: Promise<string> }>> { |
There was a problem hiding this comment.
Probably leave this inferred?
| * @see {@link generateSpecifier} to create the specifier argument. | ||
| */ | ||
| public async claimReward(request: ClaimRewardsRequest) { | ||
| for (let i = 0; i < 5; i++) { |
There was a problem hiding this comment.
@rickyrombo what do you think about adding retries on the whole thing for now? even w the fix below there are still some block height exceeded or NotEnoughSigners errors that eventually resolve for some reason.
There was a problem hiding this comment.
I want to solve these problems more generally and more deeply understand why they happen. I think this should work but it means every time we interact with Solana in a pattern like this we have to do fairly aggressive client-side retries, which usually is quite wasteful as we already have many, many retries on the relay side.
Before these changes, we did have client side retries and the results weren't much better - we just failed over and over and the experience was actually longer for the user and still failed. Without understanding why we get the error and blindly retrying, we may run into this again...
I think we should roll out your initial change w/o retries and see what happens in our analytics, and see if NotEnoughSigners errors go down
There was a problem hiding this comment.
yeaa i hear you, removed the retries. just unfortunate that i think it'll still be unstable but let's see and sync if it's still an issue.
There was a problem hiding this comment.
Yeah the irony is sometimes these "failures" aren't actually real. Like "TransactionExpiredBlockheightExceededError" sometimes occurs even though the transaction actually does make it through. I wonder if there's some issue in the assumptions made by the confirmTransaction function? This one in particular has been boggling my brain. If the transaction expires, that means the blockhash is out of the list of onchain blockhashes that are valid, which means should the transaction now get processed, it will be rejected with "blockhashnotfound". So how are these transactions succeeding? Are the blockhashes still lingering around past their expiry? Is there a race condition with the expiry and the confirmation? What's going on??
There was a problem hiding this comment.
{"error":"TransactionExpiredBlockheightExceededError: Signature 5da55Xj5cvhRw8xZZMgMZfPZerFnfHW1hLgv2DsarhwC97LWKh56FGGLqbDvekzwechuZVPopisMfTioygLvZGun has expired: block height exceeded.","signature":"5da55Xj5cvhRw8xZZMgMZfPZerFnfHW1hLgv2DsarhwC97LWKh56FGGLqbDvekzwechuZVPopisMfTioygLvZGun"}
for example, from amplitude, succeeded: https://explorer.solana.com/tx/5da55Xj5cvhRw8xZZMgMZfPZerFnfHW1hLgv2DsarhwC97LWKh56FGGLqbDvekzwechuZVPopisMfTioygLvZGun
rickyrombo
left a comment
rickyrombo
left a comment
There was a problem hiding this comment.
Don't forget to add a changeset before merging!
[1ba94b8] Reland "[PAY-3357] Migrate Notifications to SDK" (#9539) Marcus Pasell [c33b4e3] Revert "[PAY-3357] Migrate Notifications to SDK" (#9538) Marcus Pasell [b8c360e] [PAY-3357] Migrate Notifications to SDK (#9516) Marcus Pasell [eaef9fd] [QA-1498] Fix long press on DMs (#9533) JD Francis [4bbfeff] [PAY-3373] Purchasers endpoint (#9525) Reed [72ff6a3] [C-4909] Improve track metadata list, fix album backlinks (#9531) Dylan Jeffers [e5248b7] [QA-1515] Remove dismiss on leave from popup (#9527) JD Francis [46305bb] [PAY-2918] Migrate track endpoints to SDK - Round 3 (#9522) Randy Schott [b957a52] Fix challenge api tx confirming (#9526) Isaac Solo [6999b53] [QA-1519] Fix buyer metadata caching (#9512) Isaac Solo [7f17c95] Hook up remixers count to web blast UI (#9523) Reed [75ab270] [PAY-3359] Chat blast thread UI on sender side (#9515) Reed [93e3ac9] Mobile comment fixes (#9507) Sebastian Klingler [c81ed5d] [QA-1499] Fix long usernames overflowing manager mode button (#9519) JD Francis [7763deb] Add react-router dependency (#9520) Isaac Solo [6de56bd] Move trpc to common (#9518) Dylan Jeffers [76d4095] Active styles for web chat blasts (#9517) Reed [3464a2a] Add row and column props to flex (#9505) Dylan Jeffers [e6b1da1] [C-4902] Add harmony checkbox (#9510) Dylan Jeffers [0fcf9e4] [PAY-3361] ChatBlastSelectAudienceScreen (#9502) Andrew Mendelsohn [2566eb8] [PAY-3228] Implement composer unfurl in mobile (#9504) Raymond Jacobson [b5bf00a] [QA-1210] Fix popup scrolling issues (#9508) JD Francis [5349315] Update logic for allowing chat blast sending (#9509) Reed [51391d9] [PAY-3362] Move link resolver to common hook (#9503) Raymond Jacobson [45dc83d] Add feature flag refresh (#9506) Dylan Jeffers [3af4e69] [PAY-2918] Migrate track endpoints to SDK, Round 2 (#9494) Randy Schott [78897c4] [PAY-3355] Address link preview / unfurl QA items (#9492) Raymond Jacobson [c2a9387] [PAY-3366] Fix CTA hidden on small screens (#9499) Andrew Mendelsohn [e6dbb7a] Fix CTA empty states reversed web/mobile (#9498) Andrew Mendelsohn [f3d7825] Remove listenUserId from chat api (#9479) Reed [d55dbb1] [PAY-3360] Chat Blast CTA (mobile) (#9491) Andrew Mendelsohn [adb99e3] Fix challenge claim notification amount (#9485) Isaac Solo [d4518ce] [C- 4926] Add Menu/MenuItem and update Select (#9427) Dylan Jeffers [b914ade] [PAY-2918] Migrate tracks endpoints to SDK - round 1 (#9474) Randy Schott [893ed42] Revamp local solana-test-validator (#9482) Marcus Pasell [7ff4ab0] Separate saga top level error handling (#9458) Raymond Jacobson [06a19e6] Remote config for chat blast tier requirement (#9490) Reed [2b804b2] [PAY-3345] Prevent artist reacting to their own blasts (#9488) Reed [1cc1915] [C-4976] Refactor comment hooks (#9489) Sebastian Klingler [b3a4c77] [QA-1511] Fix space & arrow hotkeys not working (#9486) JD Francis [1342d1f] [C-4823] Mobile comment drawer (#9478) Sebastian Klingler [075d757] [QA-1514] Fix search filter typing (#9484) Sebastian Klingler [bfaa536] Enabling blocking users from commenting (#9469) Isaac Solo [c2d5e85] Bump mobile apps to 111 (#9481) Dylan Jeffers [5944ced] [PAY-3358] Fire chat message RPCs on blast write (#9476) Reed [83c97ed] [C-4972] Add comment sort bar (#9477) JD Francis
2 participants
Description
submitDiscoveryAttestationsshould match the return type ofsubmitAntiAbuseOracleAttestationotherwise it gets filtered out and not actually confirmed. Probably not the main reason for claiming instability but maybe part of it.How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide repro instructions & any configuration.
Ran locally against prod and confirmed that this was able to confirm then claim properly.