BM-1933: Additional Order Fulfillment Checks #1329
Conversation
github-actions
bot
changed the title
austinabell
requested review from
a team,
Wollac,
capossele,
ec2,
willemolding and
zeroecco
as code owners
|
🚀 Documentation Preview Deployment URL: https://boundless-documentation-i3fyef2o0-boundless-network.vercel.app Updated at: 2025-11-21 23:34:38 UTC |
| if let Some(log) = receipt.decoded_log::<IBoundlessMarket::PaymentRequirementsFailed>() { | ||
| let raw_error = Bytes::copy_from_slice(&log.error); | ||
| match IBoundlessMarketErrors::abi_decode(&raw_error) { | ||
| Ok(err) => Err(MarketError::PaymentRequirementsFailed(err)), | ||
| Err(_) => Err(MarketError::PaymentRequirementsFailedUnknownError(raw_error)), | ||
| } |
There was a problem hiding this comment.
Second guessing this change. Is perhaps a bit misleading to throw an error if only one of the fulfillments failed, especially since the tx didn't fail. Perhaps the sanity checks to avoid submitting if the order is fulfilled is sufficient? @mintybasil was there a strong reason why you wanted this, or just for a better error message? Wondering if okay to just warn log in these cases?
There was a problem hiding this comment.
Definitely agree this could be handled more gracefully if there are multiple fulfillments. I am a bit biased since I rarely submit batches > 1.
Im on mobile right now, but IIRC this was also to address the successful submission log that specifies the reward amounts, since that is no longer accurate.
Warn level log seems very reasonable, that makes the issue visible to operators which is all that really matters.
|
Also - this can close #1304. |
Replaces #1323 (CI)
Draft because I'd also like to add a sanity check poll on proving to ensure if an event is missed that it still doesn't continue to prove