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

add sending value along tx data #23

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

LukasJenicek
Copy link
Contributor

@LukasJenicek LukasJenicek commented Nov 19, 2024

CLI send transaction

./dist/bin.js wallet send-tx \
  --key "0x0000000000000" \
  --network "polygon" \
  --to "0xfdb42A198a932C8D3B506Ffa5e855bC4b348a712" \
  --value "1000000" \
  --data "0xa93c531700000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000e00000000000000000000000000000000000000000000000000000000000000120000000000000000000000000000000000000000000000000000000000000016000000000000000000000000000000000000000000000000000000000000001a00000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000100000000000000000000000038f0fa12ff9d87345186995d2bece9612309ba4a0000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000000"

Giving me

  ____
 / ___|  ___  __ _ _   _  ___ _ __   ___ ___
 \___ \ / _ \/ _` | | | |/ _ \ '_ \ / __/ _ \
  ___) |  __/ (_| | |_| |  __/ | | | (_|  __/
 |____/ \___|\__, |\__,_|\___|_| |_|\___\___|
                |_|

Error: missing revert data (action="estimateGas", data=null, reason=null, transaction={ "data": "0xa93c531700000000000000000
    at makeError (file:///home/lj/Projects/Sequence/sequence-cli/node_modules/.pnpm/[email protected][email protected]_utf-8-val
    at getBuiltinCallException (file:///home/lj/Projects/Sequence/sequence-cli/node_modules/.pnpm/[email protected]_bufferutil@4
    at AbiCoder.getBuiltinCallException (file:///home/lj/Projects/Sequence/sequence-cli/node_modules/.pnpm/[email protected]_buf
    at JsonRpcProvider.getRpcError (file:///home/lj/Projects/Sequence/sequence-cli/node_modules/.pnpm/[email protected]_bufferut
    at file:///home/lj/Projects/Sequence/sequence-cli/node_modules/.pnpm/[email protected][email protected][email protected]
    at process.processTicksAndRejections (node:internal/process/task_queues:105:5) {
  code: 'CALL_EXCEPTION',
  action: 'estimateGas',
  data: null,
  reason: null,
  transaction: {
    to: '0xfdb42A198a932C8D3B506Ffa5e855bC4b348a712',
    data: '0xa93c531700000000000000000000000000000000000000000000000000000000000000a00000000000000000000000000000000000000000000000000000000000000000016000000000000000000000000000000000000000000000000000000000000001a00000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000100000000000000000000000038f0fa12ff9d87345186995d2bece9612309ba4a0000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000000',
    from: '0xb0f712599786cac4B0e4616fB0ae2e29B955794E'
  },
  invocation: null,
  revert: null,
  shortMessage: 'missing revert data',
  info: {
    error: { code: -32000, message: 'execution reverted' },
    payload: {
      method: 'eth_estimateGas',
      params: [Array],
      id: 3,
      jsonrpc: '2.0'
    }
  }
}

Get Orders on Indexer:

curl --location 'https://dev-polygon-indexer.sequence.app/rpc/Indexer/GetOrderbookOrders' \
--header 'Content-Type: application/json' \
--data '{
    "orderbookContractAddress": "0xfdb42A198a932C8D3B506Ffa5e855bC4b348a712",
    "collectionAddress": "0x602D5DC17490794267C7FA5F58A453EB9159A86D",
    "orderStatuses": [
        "OPEN",
        "CLOSED",
        "CANCELLED"
    ]
}'

Response

{
    "page": {
        "page": 2,
        "pageSize": 30,
        "more": false
    },
    "orders": [
        {
            "orderId": "0",
            "tokenContract": "0x602d5dc17490794267c7fa5f58a453eb9159a86d",
            "tokenId": "8",
            "isListing": true,
            "quantity": "1",
            "quantityRemaining": "1",
            "currencyAddress": "0x3c499c542cef5e3811e1192ce70d8cc03d5c3359",
            "pricePerToken": "1000000",
            "expiry": "1732182160",
            "orderStatus": "CANCELLED",
            "createdBy": "0xb0f712599786cac4b0e4616fb0ae2e29b955794e",
            "blockNumber": 64270151,
            "orderbookContractAddress": "0xfdb42a198a932c8d3b506ffa5e855bc4b348a712",
            "createdAt": 0
        },
        {
            "orderId": "1",
            "tokenContract": "0x602d5dc17490794267c7fa5f58a453eb9159a86d",
            "tokenId": "8",
            "isListing": true,
            "quantity": "1",
            "quantityRemaining": "1",
            "currencyAddress": "0x0000000000000000000000000000000000000000",
            "pricePerToken": "1000000",
            "expiry": "1732458897",
            "orderStatus": "OPEN",
            "createdBy": "0xb0f712599786cac4b0e4616fb0ae2e29b955794e",
            "blockNumber": 64280154,
            "orderbookContractAddress": "0xfdb42a198a932c8d3b506ffa5e855bc4b348a712",
            "createdAt": 1731598679
        },
        {
            "orderId": "2",
            "tokenContract": "0x602d5dc17490794267c7fa5f58a453eb9159a86d",
            "tokenId": "8",
            "isListing": true,
            "quantity": "1",
            "quantityRemaining": "1",
            "currencyAddress": "0x0000000000000000000000000000000000000000",
            "pricePerToken": "1000000",
            "expiry": "1732458897",
            "orderStatus": "OPEN",
            "createdBy": "0xb0f712599786cac4b0e4616fb0ae2e29b955794e",
            "blockNumber": 64280154,
            "orderbookContractAddress": "0xfdb42a198a932c8d3b506ffa5e855bc4b348a712",
            "createdAt": 1731598679
        },
        {
            "orderId": "3",
            "tokenContract": "0x602d5dc17490794267c7fa5f58a453eb9159a86d",
            "tokenId": "8",
            "isListing": true,
            "quantity": "1",
            "quantityRemaining": "1",
            "currencyAddress": "0x0000000000000000000000000000000000000000",
            "pricePerToken": "1000000",
            "expiry": "1732458897",
            "orderStatus": "OPEN",
            "createdBy": "0xb0f712599786cac4b0e4616fb0ae2e29b955794e",
            "blockNumber": 64476227,
            "orderbookContractAddress": "0xfdb42a198a932c8d3b506ffa5e855bc4b348a712",
            "createdAt": 1732023428
        }
    ]
}

@ScreamingHawk
Copy link

How was this request encoded? It looks like the format is incorrect.

$ cast abi-decode "acceptRequestBatch(uint256[] calldata,uint256[] calldata,address[] calldata,uint256[] calldata,address[] calldata)" 0xa93c531700000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000e00000000000000000000000000000000000000000000000000000000000000120000000000000000000000000000000000000000000000000000000000000016000000000000000000000000000000000000000000000000000000000000001a00000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000100000000000000000000000038f0fa12ff9d87345186995d2bece9612309ba4a0000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000000
Error: no data was decoded

@LukasJenicek
Copy link
Contributor Author

Considering this value 0xa93c531700000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000e00000000000000000000000000000000000000000000000000000000000000120000000000000000000000000000000000000000000000000000000000000016000000000000000000000000000000000000000000000000000000000000001a00000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000100000000000000000000000038f0fa12ff9d87345186995d2bece9612309ba4a0000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000000

I was able to decode it using our DecodeContractCall..

Response:

{
    "call": {
        "signature": "acceptRequestBatch(uint256[],uint256[],address[],uint256[],address[])",
        "function": "acceptRequestBatch",
        "args": [
            {
                "name": null,
                "type": "uint256[]",
                "value": [
                    "1"
                ]
            },
            {
                "name": null,
                "type": "uint256[]",
                "value": [
                    "1"
                ]
            },
            {
                "name": null,
                "type": "address[]",
                "value": [
                    "0x38F0FA12Ff9d87345186995D2Bece9612309BA4a"
                ]
            },
            {
                "name": null,
                "type": "uint256[]",
                "value": [
                    "0"
                ]
            },
            {
                "name": null,
                "type": "address[]",
                "value": [
                    "0x0000000000000000000000000000000000000000"
                ]
            }
        ]
    }
}

@LukasJenicek
Copy link
Contributor Author

LukasJenicek commented Nov 20, 2024

Transactor: https://github.com/0xsequence/marketplace-api/blob/6597292e2cd784ffe3ea9860c5ce5cc66cfeed93/lib/contracts/sequencemarketplace/gen/sequencemarketplace/sequencemarketplace.gen.go#L429

package main

import (
	"cmp"
	"context"
	"fmt"
	"log"
	"math/big"
	"os"

	"github.com/0xsequence/ethkit/go-ethereum/accounts/abi/bind"
	"github.com/0xsequence/ethkit/go-ethereum/common"
	"github.com/0xsequence/ethkit/go-ethereum/common/hexutil"
	"github.com/0xsequence/ethkit/go-ethereum/core/types"
	"github.com/0xsequence/go-sequence/lib/prototyp"
	"github.com/0xsequence/marketplace-api/config"
	"github.com/0xsequence/marketplace-api/core"
	"github.com/0xsequence/marketplace-api/lib/contracts/sequencemarketplace"

	contract "github.com/0xsequence/marketplace-api/lib/contracts/sequencemarketplace/gen/sequencemarketplace"
)

func main() {
	ctx := context.Background()

	cfg, err := config.LoadConfig(cmp.Or(os.Getenv("CONFIG"), "/home/lj/Projects/Sequence/marketplace/etc/config.conf"))
	if err != nil {
		log.Fatal(err)
	}

	c, err := core.Init(ctx, cfg, "playground")
	if err != nil {
		log.Fatal(err)
	}

	transactor, err := contract.NewSequenceMarketplaceTransactor(
		common.HexToAddress(sequencemarketplace.SequenceMarketplaceV2Address),
		c.Providers.GetByChainID(137),
	)
	if err != nil {
		log.Fatal(err)
	}

	requestIds := []*big.Int{prototyp.NewBigInt(1).Int()}
	quantities := []*big.Int{prototyp.NewBigInt(1).Int()}
	recipients := []common.Address{common.HexToAddress("0x38F0FA12Ff9d87345186995D2Bece9612309BA4a")}

	opts := &bind.TransactOpts{
		Signer: func(_ common.Address, tx *types.Transaction) (*types.Transaction, error) {
			return tx, nil
		},
		Context:  ctx,
		NoSend:   true,
		Value:    big.NewInt(1000000),
		GasLimit: 1,
	}

	tx, err := transactor.AcceptRequestBatch(opts, requestIds, quantities, recipients, []*big.Int{}, []common.Address{})
	if err != nil {
		log.Fatalf("accept request batch: %v", err)
	}

	fmt.Printf("%s", hexutil.Encode(tx.Data()))
	fmt.Fprintf(os.Stderr, "%s", hexutil.Encode(tx.Data()))
}

@ScreamingHawk
Copy link

Found the issue!

  function acceptRequest(
    uint256 requestId,
    uint256 quantity,
    address recipient,
    uint256[] calldata additionalFees,
    address[] calldata additionalFeeRecipients
  )
    external payable;

  function acceptRequestBatch(
    uint256[] calldata requestIds,
    uint256[] calldata quantities,
    address[] calldata recipients,
    uint256[] calldata additionalFees,
    address[] calldata additionalFeeRecipients
  )
    external;

The acceptRequestBatch function doesn't support accepting payments in the native token. This is intentional due to the way we handle calculating fees for payments. It will be something we support in later versions.

@VojtechVitek
Copy link

@ScreamingHawk how much work would it be to add support for acceptRequestBatch() with native tokens in v2?

I spoke with Lukas and it sounds like we're using acceptRequestBatch() to buy multiple items in a single transaction in v1 right now. But with v2 and native tokens, we can't do the same, since we can't use the batch transaction with acceptRequestBatch() as of now.

We're unsure how to resolve this issue - how to buy multiple items in the cart with native currency. If we split each item to its own transaction, it'd likely lead to a bad UX, since user would needs to sign multiple buy transactions individually.

What's the recommended next steps?

cc @taylanpince @LukasJenicek

@ScreamingHawk
Copy link

Good news. The marketplace is upgradeable so we should be able to perform an upgrade with the added functionality without changing the contract address. This would require some testing and would be a couple days of work to get it deployed on all chains.

@ScreamingHawk
Copy link

0xsequence/marketplace-contracts#7 Marketplace update for reference

@ScreamingHawk
Copy link

ScreamingHawk commented Dec 3, 2024

0xsequence/marketplace-contracts#8
Went with a simpler helper contract for accepting batches of requests in native currency.

Will be deployed at 0x6166c1952c54dEd6b070B4616797E61b6c48A117. Currently on base sepolia (let me know if you want other chains for testing).
ABI:

  {
    type: 'function',
    name: 'acceptRequestBatch',
    inputs: [
      { name: 'market', type: 'address', internalType: 'contract ISequenceMarketFunctions' },
      { name: 'requestIds', type: 'uint256[]', internalType: 'uint256[]' },
      { name: 'quantities', type: 'uint256[]', internalType: 'uint256[]' },
      { name: 'recipients', type: 'address[]', internalType: 'address[]' },
      { name: 'additionalFees', type: 'uint256[]', internalType: 'uint256[]' },
      { name: 'additionalFeeRecipients', type: 'address[]', internalType: 'address[]' }
    ],
    outputs: [],
    stateMutability: 'payable'
  }

Note this doesn't work with requests that use ERC20s. This should only be used for native token requests.

@VojtechVitek
Copy link

0xsequence/marketplace-contracts#8

@ScreamingHawk Cool, thanks for the update! 🎉

Note this doesn't work with requests that use ERC20s. This should only be used for native token requests.

Is there anything we can do to support both ERC20 and native tokens in a single transaction for both types?

If not, I wonder what our Marketplace is supposed to do -- do we need to group native vs. ERC-20 collectibles into two different cart transactions?

cc @LukasJenicek @AlexanderKolberg @taylanpince @SamueleA

@taylanpince
Copy link
Contributor

@ScreamingHawk Thank you for the update, that's great.
@VojtechVitek Yes we will have to separate ERC20 and native token transactions. This will be a part of the payment options API iteration, to be able to support cart transactions. We will need logic for grouping items.

@LukasJenicek Do you mind finalizing this PR and resolving conflicts when you get a chance? I would like to merge this in, it's a handy addition.

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.

4 participants