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

fix(request_vc_inner): remove duplicated String encoding #2509

Merged

Conversation

jonalvarezz
Copy link
Contributor

Context

There is a duplicated encoding to Error strings on request_vc_inner which led to the issue described in P-529:

See:

Testing Evidences

RcpReturnValue response after the change:

{value: '0x504e6f20656c696769626c65206964656e74697479', do_watch: false, status: 'Error'}

Decoding 0x504e6f20656c696769626c65206964656e74697479 gives No eligible identity

Copy link

linear bot commented Feb 20, 2024

Copy link
Collaborator

@Kailai-Wang Kailai-Wang left a comment

Choose a reason for hiding this comment

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

Thanks!

I see we call compute_hex_encoded_return_error in the outer loop already

@Kailai-Wang
Copy link
Collaborator

Btw it seems to be a generic problem other than No eligible identity only, our ts-test should have discovered this actually.

@Kailai-Wang Kailai-Wang merged commit c52a49f into dev Feb 20, 2024
31 checks passed
@jonalvarezz
Copy link
Contributor Author

jonalvarezz commented Feb 20, 2024

Btw it seems to be a generic problem other than No eligible identity only,

@Kailai-Wang, Yes, it was not exclusive of No elegible identity but any error during the vc creation. I used that one as example.

Here is a data provider error:

RpcReturnValue;

{
  value: '0xdd084661696c656420746f206275696c6420617373657274696f6e2064756520746f3a205265717565737456434661696c6564284252433230416d6f756e74486f6c6465722c204461746150726f76696465724572726f7228426f756e646564566563285b37312c203130312c203131302c203130352c203130352c2036382c2039372c203131362c2039372c2036392c203131342c203131342c203131312c203131342c2034302c2033342c2037312c203130312c203131302c203130352c203130352c2036382c2039372c203131362c2039372c2033322c203131342c203130312c203131352c203131322c203131312c203131302c203131352c203130312c2033322c203130312c203131342c203131342c203131312c203131342c2035382c2033322c2037302c2039372c203130352c203130382c203130312c203130302c2033322c203131362c203131312c2033322c203130302c203130312c203131352c203130312c203131342c203130352c2039372c203130382c203130352c203132322c203130312c2033322c203130302c2039372c203131362c2039372c2033322c203131362c203131312c2033322c203131352c203131362c203131342c203131372c2039392c203131362c2033322c2034302c203130352c203131302c2033322c2037312c2036392c2038342c2033322c203131312c203131342c2033322c2038302c2037392c2038332c2038342c2033322c203131342c203130312c203131352c203131322c203131315d2c20313030292929',
  do_watch: false,
  status: 'Error'
}

Decoding (using the fix on this PR):

RequestVCFailed(BRC20AmountHolder, DataProviderError(BoundedVec([71, 101, 110, 105, 105, 68, 97, 116, 97, 69, 114, 114, 111, 114, 40, 34, 71, 101, 110, 105, 105, 68, 97, 116, 97, 32, 114, 101, 115, 112, 111, 110, 115, 101, 32, 101, 114, 114, 111, 114, 58, 32, 70, 97, 105, 108, 101, 100, 32, 116, 111, 32, 100, 101, 115, 101, 114, 105, 97, 108, 105, 122, 101, 32, 100, 97, 116, 97, 32, 116, 111, 32, 115, 116, 114, 117, 99, 116, 32, 40, 105, 110, 32, 71, 69, 84, 32, 111, 114, 32, 80, 79, 83, 84, 32, 114, 101, 115, 112, 111], 100)))

The DataProviderError detail could be better, but that's another issue, not related to this fix.

our ts-test should have discovered this actually.

I checked before opening, but current tests don't check errors at this detail. I agree that they should.

@zhouhuitian zhouhuitian deleted the p-529-direct-response-of-request_vc-has-wrong-encoded-error branch February 21, 2024 01:26
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