chore: prepare release #941
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Fix + improve the confirm transaction e2e test The test was incorrect because it used the same transaction hash repeatedly, leading to assertions being made on an already confirmed transaction instead of a newly proposed one. By adding a timestamp to the transaction data, we ensure that each transaction hash is unique. Additionally, the test incorrectly called the `isValidSignature` function with a transaction hash. This function is intended only for messages, causing the contract call to fail consistently. However, the error was not properly caught; the function merely returns `false` when an error occurs. With the migration to viem, the `isValidSignature` contract call sometimes takes too long to fail, resulting in occasional test timeouts. By removing the incorrect call, we prevent those timeout errors. * Fix tests by using another Safe address that is not used by the confirmTransaction test
improve check scripts
Pull Request Test Coverage Report for Build 10349994982Details
💛 - Coveralls |
DaniSomoza
approved these changes
Aug 12, 2024
leonardotc
reviewed
Aug 14, 2024
| @@ -32,8 +36,9 @@ describe('proposeTransaction', () => { | |||
| it('should allow to create and confirm transactions signature using a Safe signer', async () => { | |||
| const safeTransactionData: SafeTransactionDataPartial = { | |||
| to: safeAddress, | |||
| value: '100000000000000000', // 0.01 ETH | |||
| data: '0x' | |||
| value: '10000000000000000', // 0.01 ETH | |||
There was a problem hiding this comment.
Is the comment on the right-hand side still correct? You could use the viem#parseEther and remove the comment altogether.
leonardotc
reviewed
Aug 14, 2024
| .sort((a, b) => Number(a.chainId - b.chainId)) | ||
| .map( | ||
| (network, index) => | ||
| ` { chainId: ${network.chainId}n, shortName: '${network.shortName}' }${index === networks.length - 1 ? '' : ','}` |
There was a problem hiding this comment.
The problem with this thing is that, because we are assembling bits of content in different statements, its hard to understand the outgoing structure. Life would be much kinder to us if it looked more like "stuffing" a template (if possible)... e.g.:
`
export const networks: NetworkShortName[] = [
${sortedNetworks.map(network => `
{
chainId: '${network.name}',
shortName: '${network.shortName}'
}`).join(',')}
]
`
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What it solves
Prepare release to be merged into main
Versions
Main changes
@safe-global/safe-deploymentsto the latest version #936