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

access list test example [example] #772

Closed
wants to merge 7 commits into from
Closed

access list test example [example] #772

wants to merge 7 commits into from

Conversation

winsvega
Copy link
Collaborator

@winsvega winsvega commented Jan 23, 2021

@holiman
Copy link
Contributor

holiman commented Feb 3, 2021

Reposting this here: I think the "data" : ":label somelabel :raw 0x1122", is really not a good idea. I think it's fine to have

 "data" : "0x..",
 "_dataCommment": "Foo bar bazonk",
 "accessList" : [ ...] 
"_anotherComment": "Baz bar",

But inserting logic into the data-field is really making things too complex, and doesn't play well with json-parsers: e.g. in golang, our parsing is typed. As in, we define what the type of a specific input is, and having some mixed, loosely defined meta-format with :-delimited fields just adds another level of quirkyness

"0x3344"
],
"accessLists" : {
"0" : [
Copy link
Member

@MariusVanDerWijden MariusVanDerWijden Feb 8, 2021

Choose a reason for hiding this comment

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

I don't really know how to unmarshal this properly as the key depends on the length of data. I would suggest to drop the index and just make accessLists an array like data with the same length and leave elements empty if you don't want to have an access list for it. cc @winsvega @holiman

e.g.

type stTransaction struct {
	GasPrice    *big.Int       `json:"gasPrice"`
	Nonce       uint64         `json:"nonce"`
	To          string         `json:"to"`
	Data        string         `json:"data"`
	AccessLists []types.AccessList `json:"accessLists,omitempty"`
	GasLimit    []uint64       `json:"gasLimit"`
	Value       []string       `json:"value"`
	PrivateKey  []byte         `json:"secretKey"`
}

type AccessList []AccessTuple  `json:"omitempty"`

type AccessTuple struct {
	Address     *common.Address `json:"address"        gencodec:"required"`
	StorageKeys []*common.Hash  `json:"storageKeys"    gencodec:"required"`
}

Copy link
Member

@MariusVanDerWijden MariusVanDerWijden Feb 8, 2021

Choose a reason for hiding this comment

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

So basically my idea would be something like this:

"transaction" : {
            "data" : [
                "0x1122",
                "0x3344",
                "0x3234"
            ],
            "accessLists" : [
                    {
                        "address" : "0x0000000000000000000000000000000000001337",
                        "storageKeys" : [
                            "0x0000000000000000000000000000000000000000000000000000000000000001",
                            "0x0000000000000000000000000000000000000000000000000000000000000002"
                        ]
                    },
                    {
                    
                    },
                    {
                        "address" : "0x0000000000000000000000000000000000001337",
                        "storageKeys" : [
                            "0x0000000000000000000000000000000000000000000000000000000000000001",
                            "0x0000000000000000000000000000000000000000000000000000000000000002",
                            "0x0000000000000000000000000000000000000000000000000000000000000002"
                        ]
                    },
            ],
            "gasLimit" : [
                "0x061a80"
            ],
            "gasPrice" : "0x01",
            "nonce" : "0x00",
            "secretKey" : "0x45a915e4d060149eb4365960e6a7a45f334393093061116b197e3240065ff2d8",
            "to" : "0x095e7baea6a6c7c4c2dfeb977efac326af552d87",
            "value" : [
                "0x0186a0"
            ]
        }

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree about that. To clarify: {} can be used for empty list, and null to signify that this is a legacy-tx.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't really know how to unmarshal this properly as the key depends on the length of data.

Unmarshalling shouldn't be a problem, though, it's just a map[int]AccessList. But it's still a bit convoluted

Copy link
Collaborator Author

@winsvega winsvega Feb 9, 2021

Choose a reason for hiding this comment

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

you think just null for the transaction with data where accessList not present is better ?

ok. it is just the matter of test format. which one is easier for you guys.

@winsvega
Copy link
Collaborator Author

winsvega commented Feb 11, 2021

I removed the map and now it is just an array (same length as data array)
{} for absense of accessList
[] for empty accessList

ori alsomst finished his tests and will generate soon on this branch
#774

@winsvega winsvega mentioned this pull request Feb 21, 2021
@winsvega
Copy link
Collaborator Author

Is there a problem with parsing {}?

Martin wants it to be explicit null

@holiman
Copy link
Contributor

holiman commented Feb 21, 2021 via email

@winsvega
Copy link
Collaborator Author

winsvega commented Feb 26, 2021

added "txbytes" field in gstate post section. so that no need to build a transaction and do the signing operation from transaction section (complicated logic)

here also a blockchain test example with access list

@winsvega winsvega changed the title access list test example access list test example [example] Mar 9, 2021
@winsvega winsvega closed this Apr 15, 2021
@winsvega winsvega deleted the accesslist branch May 7, 2021 22:04
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.

3 participants