Feature/refactor lib #84

Ptroger · 2024-02-05T02:04:05Z

No description provided.

wcalderipe · 2024-02-05T08:23:19Z

packages/transaction-request-intent/src/lib/__test__/unit/mocks.ts

@@ -249,7 +249,7 @@ const ERC20_TRANSFER_INTENT: TransferErc20 = {
  type: Intents.TRANSFER_ERC20,
  to: `eip155:137/0x031d8c0ca142921c459bcb28104c0ff37928f9ed` as AccountId,
  from: `eip155:137/${ERC20_TRANSFER_TX_REQUEST.from.toLowerCase()}` as AccountId,
-  contract: `eip155:137/${ERC20_TRANSFER_TX_REQUEST.to?.toLowerCase()}` as AccountId,
+  token: `eip155:137/${ERC20_TRANSFER_TX_REQUEST.to?.toLowerCase()}` as AccountId,


Suggested change

token: `eip155:137/${ERC20_TRANSFER_TX_REQUEST.to?.toLowerCase()}` as AccountId,

token: getAccountId(`eip155:137/${ERC20_TRANSFER_TX_REQUEST.to?.toLowerCase()}`),

Use the utility function to get an account ID to ensure the format is right on your tests.

wcalderipe · 2024-02-05T08:23:58Z

packages/transaction-request-intent/src/lib/__test__/unit/mocks.ts

@@ -267,7 +267,7 @@ const ERC721_SAFE_TRANSFER_FROM_INTENT: TransferErc721 = {
  to: `eip155:137/0xb253f6156e64b12ba0dec3974062dbbaee139f0c` as AccountId,
  from: `eip155:137/${ERC721_SAFE_TRANSFER_FROM_TX_REQUEST.from.toLowerCase()}` as AccountId,
  contract: `eip155:137/${ERC721_SAFE_TRANSFER_FROM_TX_REQUEST.to?.toLowerCase()}` as AccountId,
-  nftId: `eip155:137/erc721:${ERC721_SAFE_TRANSFER_FROM_TX_REQUEST.to}/41173` as AssetId
+  token: `eip155:137/erc721:${ERC721_SAFE_TRANSFER_FROM_TX_REQUEST.to}/41173` as AssetId


Suggested change

token: `eip155:137/erc721:${ERC721_SAFE_TRANSFER_FROM_TX_REQUEST.to}/41173` as AssetId

token: getAssetId(`eip155:137/erc721:${ERC721_SAFE_TRANSFER_FROM_TX_REQUEST.to}/41173`)

wcalderipe · 2024-02-05T08:24:24Z

packages/transaction-request-intent/src/lib/__test__/unit/mocks.ts

@@ -315,7 +315,7 @@ const ERC1155_SAFE_TRANSFER_FROM_INTENT: TransferErc1155 = {
  to: `eip155:137/0x00ca04c45da318d5b7e7b14d5381ca59f09c73f0` as AccountId,
  from: `eip155:137/${ERC1155_SAFE_TRANSFER_FROM_TX_REQUEST.from.toLowerCase()}` as AccountId,
  contract: `eip155:137/${ERC1155_SAFE_TRANSFER_FROM_TX_REQUEST.to?.toLowerCase()}` as AccountId,
-  transfers: [{ tokenId: `eip155:137/erc1155:${ERC1155_SAFE_TRANSFER_FROM_TX_REQUEST.to}/175` as AssetId, amount: '1' }]
+  transfers: [{ token: `eip155:137/erc1155:${ERC1155_SAFE_TRANSFER_FROM_TX_REQUEST.to}/175` as AssetId, amount: '1' }]


Out of curiosity, why the amount is a string?

Amount is a bigint encoded in Hex. So I decode it back to bigint and send it as a string, so that consumer decides how/if it needs to be encoded back into a number.

wcalderipe · 2024-02-05T09:26:33Z

apps/orchestration/src/data-feed/core/service/price-feed.service.ts

+      const result = safeDecode({
+        input: {
+          type: InputType.TRANSACTION_REQUEST,
+          txRequest: authzRequest.request.transactionRequest
+        }
      })


Do you think something like this would be possible?

Suggested change

const result = safeDecode({

input: {

type: InputType.TRANSACTION_REQUEST,

txRequest: authzRequest.request.transactionRequest

}

})

const result = safeDecode(authzRequest.request.transactionRequest)

// or

const result = safeDecode(authzRequest.request.message)

I guess it wouldn't be.

If you look at the shape of the safeDecode input, it's like so:
{ input, config = defaultConfig }: { input: DecodeInput; config?: Config }

config include all the user trustworthy data that will be used to decode the input.

transaction registry

contract registry

set of supported methods and their decoder (default to my current set)

What is doable is determining the InputType, and then do something like this:

safeDecode({ input })

Or

safeDecode({ input, config })

wcalderipe · 2024-02-05T09:28:46Z

...ages/transaction-request-intent/src/lib/decoders/transaction/native/NativeTransferDecoder.ts

-    }
-    return intent
+  return {
+    to: toAccountIdLowerCase({


Maybe you can use the toAccountId again? I changed it last week to respect the given format. In other words, if you send checksummed address you should get checksummed ID, or if you send lower case address you'll get lower case ID

Suggested change

to: toAccountIdLowerCase({

to: toAccountId({

wcalderipe · 2024-02-05T09:29:26Z

packages/transaction-request-intent/src/lib/decoders/decode.ts

+        payload: input.raw.rawData
+      }
+    default:
+      throw new Error('Invalid input type')


Suggested change

throw new Error('Invalid input type')

throw new DecodeError('Invalid input type')

Yes changing those

wcalderipe · 2024-02-05T09:29:37Z

...ansaction-request-intent/src/lib/decoders/transaction/interaction/ApproveAllowanceDecoder.ts

-    super(input)
-    this.#input = input
+  if (!isSupportedMethodId(methodId)) {
+    throw new Error('Unsupported methodId')


Suggested change

throw new Error('Unsupported methodId')

throw new DecodeError('Unsupported methodId')

wcalderipe · 2024-02-05T09:29:50Z

...ansaction-request-intent/src/lib/decoders/transaction/interaction/ApproveAllowanceDecoder.ts

-    }
+  const params = extract(supportedMethods, data, methodId) as ApproveAllowanceParams
+  if (!params) {
+    throw new Error('Params do not match ERC20 transfer methodId')


Suggested change

throw new Error('Params do not match ERC20 transfer methodId')

throw new DecodeError('Params do not match ERC20 transfer methodId')

wcalderipe

I left some comments. Nothing critical or blocking your PR.

If you CMD/CTRL + F "throw new Error" you'll notice a bunch of pending domain errors yet 😉

samteb · 2024-02-05T17:02:27Z

packages/transaction-request-intent/src/lib/intent.types.ts

  message: string
 }

 export type SignRawMessage = {
  type: Intents.SIGN_RAW_MESSAGE
  from: AccountId
-  message: string


@Ptroger as discussed, you removed the wrong data

* changed interface * changed intent naming, changed usage of lib in apps * format and lint * Update rego criteria * changed interface * used decoderErros * fix --------- Co-authored-by: samuel <samuel@narval.xyz>

Ptroger marked this pull request as ready for review February 5, 2024 02:11

Ptroger requested review from wcalderipe and samteb February 5, 2024 02:11

wcalderipe reviewed Feb 5, 2024

View reviewed changes

samteb reviewed Feb 5, 2024

View reviewed changes

samteb approved these changes Feb 6, 2024

View reviewed changes

Ptroger force-pushed the feature/refactor-lib branch from 22a320b to 6049c45 Compare February 6, 2024 16:09

Ptroger and others added 6 commits February 7, 2024 08:32

changed interface

c797337

changed intent naming, changed usage of lib in apps

9ad91dc

format and lint

37c9982

Update rego criteria

dd1fd9e

changed interface

0948a99

used decoderErros

744b769

Ptroger force-pushed the feature/refactor-lib branch from 6049c45 to 744b769 Compare February 7, 2024 12:32

fix

27754b1

Ptroger merged commit bbf8d61 into main Feb 7, 2024
2 checks passed

Ptroger deleted the feature/refactor-lib branch February 7, 2024 14:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/refactor lib #84

Feature/refactor lib #84

Ptroger commented Feb 5, 2024

wcalderipe Feb 5, 2024

wcalderipe Feb 5, 2024

wcalderipe Feb 5, 2024

Ptroger Feb 5, 2024

wcalderipe Feb 5, 2024

Ptroger Feb 5, 2024

wcalderipe Feb 5, 2024

Ptroger Feb 5, 2024

wcalderipe Feb 5, 2024

Ptroger Feb 5, 2024

wcalderipe Feb 5, 2024

wcalderipe Feb 5, 2024

wcalderipe left a comment

samteb Feb 5, 2024

	token: `eip155:137/${ERC20_TRANSFER_TX_REQUEST.to?.toLowerCase()}` as AccountId,
	token: getAccountId(`eip155:137/${ERC20_TRANSFER_TX_REQUEST.to?.toLowerCase()}`),

	token: `eip155:137/erc721:${ERC721_SAFE_TRANSFER_FROM_TX_REQUEST.to}/41173` as AssetId
	token: getAssetId(`eip155:137/erc721:${ERC721_SAFE_TRANSFER_FROM_TX_REQUEST.to}/41173`)

	throw new Error('Invalid input type')
	throw new DecodeError('Invalid input type')

	throw new Error('Unsupported methodId')
	throw new DecodeError('Unsupported methodId')

	throw new Error('Params do not match ERC20 transfer methodId')
	throw new DecodeError('Params do not match ERC20 transfer methodId')

Feature/refactor lib #84

Feature/refactor lib #84

Conversation

Ptroger commented Feb 5, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wcalderipe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment