Allow transactions, when a configured transactionPermissionContract does not exist#8940
Allow transactions, when a configured transactionPermissionContract does not exist#8940wulfraem wants to merge 3 commits into
Conversation
…igured 'transactionPermissionContract', that does (not) exist
|
It looks like @wulfraem signed our Contributor License Agreement. 👍 Many thanks, Parity Technologies CLA Bot |
5chdn
added
A0-pleasereview 🤓
| trace!(target: "tx_filter", "transaction permission contract does not exist at current block, permitting transaction"); | ||
| return true; | ||
| } else { | ||
| trace!(target: "tx_filter", "transaction permission contract exists at current block, checking permissions"); |
There was a problem hiding this comment.
I suggest removing this trace. It will spam the log
There was a problem hiding this comment.
Makes sense. I removed the logs.
andresilva
left a comment
andresilva
left a comment
There was a problem hiding this comment.
Usually what we do is create a transition flag for the feature that sets at which block the feature is enabled.
|
This pull request was intended to fix the syncing issue, that we had with the current implementation. In the second part of my initial suggested a config similar to the config of the Multi-Set Validator-Set. This would allow to swap the permission contracts over time without creating multiple transitions for each new contract. |
5chdn
added
A4-gotissues 💥
|
I'm not overly familiar with the transaction permissioning, but what @andresilva suggests is that you either have set contract enabled right from the start (would make this PR obsolete) or you want to make it highly configurable by defining some transaction permission contract "transition" logic allowing users to specify at which block which contract is being used. |
5chdn
added
the
A3-stale 🍃
|
Marking as stale, let me know if you have any further questions. |
5chdn
added
the
A1-onice 🌨
5chdn
removed
the
A1-onice 🌨
|
Replaced by #9275. |
5 participants
We wanted to use the
transactionPermissionContractand deployed a matching contract. Then added the contract to the chain specification. Clients, that already had a synced chain were able to sync, but clients, that performed a full sync, stopped at the first transaction (in our case at block 5), because the configuredtransactionPermissionContractdid not exist at this time. If the contract does not exist, the check defaults to "not permitted" due to an error, when calling the non-existing contract, this pull request checks if the contract exists and defaults to true in such a case.This fixes our issue, but i guess a useful extension to the
transactionPermissionContractlogic would be to use a similar config format like the Multi-Set Validator-Set and define starting blocks for valid permission contracts. This would allow to change the contracts and still be able to sync the chain.